[DVDnav-discuss] [PATCH] Initialize dvd_reader_t struct in DVDOpenImageFile

[Richard Hulme]

On 01/02/14 15:21, Dominik 'Rathann' Mierzejewski wrote:
> Hello Richard,
>
> On Sunday, 26 January 2014 at 17:13, Richard Hulme wrote:
>> Hi,
>>
>> This is a patch I found in MythTV's copy of libdvdread that seems
>> like it could/should be upstream too.  It ensures that the
>> dvd_reader_t struct is initialized (e.g. the css_state field is
>> currently undefined if 'have_css' is false).
>
> Thanks for forwarding the patch.
>
> If I understand correctly, with that memset in place, some of the
> subsequent assignments
>    dvd->path_root = NULL;
> [...]
>    dvd->udfcache = NULL;
> [...]
>    dvd->css_title = 0;
>
> are not necessary anymore. Actually, that case of leaving css_state
> uninitialized is a genuine bug, but I'm not sure if using memset
> is the right solution. Does anyone else have thoughts on this case?

Hi Dominik,

If I had written that patch myself, I think I might have just gone with 
explicitly initialising have_css to zero, just to fit in with the rest 
of the existing code and to keep the changes to a minimum.

The two other possibilities I see are either staying with memset or 
using calloc instead of malloc/memset.  In either case the explicit 
initialisations to zero/NULL are not necessary.

To my eyes, calloc or memset have the advantage of ensuring the entire 
structure is initialised, but there is only have_css which is not 
currently being initialised and the structure is unlikely to grow, so 
that argument in favour of calloc or memset is not as strong as it could 
be.  Also, not all fields should be zero (but only 3 from 7).

In the end, I think it comes down to personal preference, but I'm happy 
to adapt the patch as necessary.

Richard.