[DVDnav-discuss] Libdvdread misses hidden files and causes segfaults to calling programs

[Erik Hovland]

>>> This patch has been submitted twice already, but I like my version
>>> better.  The other 2 unnecessarily truncate the
>>> string when something is found in the high byte.  This fills that string
>>> the same as pre-patch, but returns a code
>>> indicating that junk was found in the MSB.
>>
>> I[n] that case I don't see the point in making things hard to review
>> by rewriting the whole function.
>> Why not just
>> Index: dvd_udf.c
>> ===================================================================
>> --- dvd_udf.c   (revision 1233)
>> +++ dvd_udf.c   (working copy)
>> @@ -329,16 +329,17 @@
>>  static int Unicodedecode( uint8_t *data, int len, char *target )
>>  {
>>    int p = 1, i = 0;
>> +  int err = 0;
>>
>>    if( ( data[ 0 ] == 8 ) || ( data[ 0 ] == 16 ) ) do {
>> -    if( data[ 0 ] == 16 ) p++;  /* Ignore MSB of unicode16 */
>> +    if( data[ 0 ] == 16 ) err |= data[ p++ ];  /* Ignore MSB of unicode16
>> */
>>      if( p<  len ) {
>>        target[ i++ ] = data[ p++ ];
>>      }
>>    } while( p<  len );
>>
>>    target[ i ] = '\0';
>> -  return 0;
>> +  return !err;
>>  }
>>
>>  static int UDFDescriptor( uint8_t *data, uint16_t *TagID )
>
> In that case, you need to observe the return value in UDFDescriptor, which
> isn't done currenty:
>
>  static int UDFDescriptor( uint8_t *data, uint16_t *TagID )
> @@ -490,7 +501,9 @@
>   L_FI = GETN1(19);
>   UDFLongAD(&data[20], FileICB);
>   L_IU = GETN2(36);
> -  if (L_FI) Unicodedecode(&data[38 + L_IU], L_FI, FileName);
> +  if (L_FI) {
> +    if (!Unicodedecode(&data[38 + L_IU], L_FI, FileName)) FileName[0] =
> '\0';
> +  }
>   else FileName[0] = '\0';
>   return 4 * ((38 + L_FI + L_IU + 3) / 4);
>  }

I don't think Reimar was actually submitting a patch but rather making
a critique of the Unicodedecode() part of the patch.

Which means that it would be handy if another patch were submitted that
made the small change that Reimar suggests along w/ the additional
conditional that is required to make the right decision on the
decoded value. Or annotate the original patch enough that it is
much clearer what is going on.

If John is up for it this is a good test of his newly minted svn write
access. ;)

E

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