[DVDnav-discuss] Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

Erik Hovland erik at hovland.org
Thu May 24 23:46:04 CEST 2012


>>> I also compared your git to my patchset.
>>> You have merged them all with the exception of part of one patch.
>>>  Unfortunately, I now can not remember the circumstances under which this
>>> small patch was useful.  I have a vague recollection that it was a rare
>>> corner case that I stumbled over, but it's been so long I really don't
>>> know.
>>>  I'm attaching the patch if you have any interest in it. It was
>>> originally
>>> part of the patch to fix vm_reset problem.
>>
>> My only issue w/ the patch is that it isn't clear why we would need to
>> change the VOB if the this->file is null. Especially confusing is the
>> snippet
>> of code inside the conditional block:
>> if(this->file) {
>>   DVDCloseFile(this->file);
>>   this->file = NULL;
>> }
>>
>> It implies that this->file being null is not a big deal. I am going to
>> leave this
>> patch out for now.
>>
>> E
>>
>
> The problem isn't with what happens when this->file is null *inside* the
> conditional.  It's a problem with what happens if the conditional *is not*
> run and this->file is null.  The file does not get opened and DVDReadBlocks
> references a null pointer.

Now that sounds like a good reason. I stuck the patch into my repo and
added some words to the comment above the conditional to mention that
opening a VOB if one isn't already open is a good idea.

Thanks again!

E

-- 
Erik Hovland
erik at hovland.org
http://hovland.org/


More information about the DVDnav-discuss mailing list