[FFmpeg-devel] [PATCH] Adding support for OSX bundles to ffmpeg command line

[Chris Marrin]

On Mar 20, 2009, at 2:37 PM, M?ns Rullg?rd wrote:

> Chris Marrin <chris at marrin.com> writes:
>
>> On Mar 20, 2009, at 9:50 AM, M?ns Rullg?rd wrote:
>>
>>> Chris Marrin <chris at marrin.com> writes:
>>>
>>>> @@ -3915,7 +3930,16 @@
>>>> {
>>>>    int i;
>>>>    int64_t ti;
>>>> -
>>>> +
>>>> +    // get the executable path
>>>> +    char* p = strrchr(argv[0], '/');
>>>> +    if (p) {
>>>> +        int length = p-argv[0];
>>>> +        executable_path = av_malloc(length+1);
>>>> +        strncpy(executable_path, argv[0], length);
>>>> +        executable_path[length] = '\0';
>>>> +    }
>>>> +
>>>
>>> There are a million ways in which this can fail.
>>
>> I can find a few:
>>
>> 1) executable_path is not set to null if the if test fails (along  
>> with
>> the corresponding check for null where used).
>> 2) The malloc can fail.
>> 3) In the unimaginably unlikely case that argv[0] contained a string
>> of greater than 2GB with a slash at the end, the length would wrap
>> 4) The executable_path is leaked (not a failure, but I can fix it)
>>
>> I can fix all those. Others?
>
> 5) argv[0] doesn't contain the (expected) path to the executable.
> There are a number of ways this can realistically happen.


The outcome of which is that a) the string looks nothing like a path  
and executable_path is empty or b) the string looks something like a  
path and executable_path contains nonsense. In either case you would  
not find the preset and fallback to existing behavior. I suppose  
argv[0] could even be null, so a check for that is needed?

Any more dangers?

-----
~Chris
chris at marrin.com