[DVDnav-discuss] dvdnav patches from handbrake project

[Nico Sabbi]

Sorry for the hand-quote, I don't have the original message right here.


John Stebbins wrote:

>To keep things organized, here's current status of these patches.
>These are pending:
>     * read-block2char.patch

This one is really disturbing me:
>Index: libdvdread/src/dvd_reader.c
>===================================================================
>--- libdvdread/src/dvd_reader.c	(revision 1194)
>+++ libdvdread/src/dvd_reader.c	(working copy)
>@@ -318,7 +318,7 @@
>    char *new_path;
>
>    /* If it doesn't start with "/dev/" or does start with "/dev/r" exit */
>-  if( !strncmp( path, "/dev/",  5 ) || strncmp( path, "/dev/r", 6 ) )
>+  if( strncmp( path, "/dev/",  5 ) || !strncmp( path, "/dev/r", 6 ) )
>      return (char *) strdup( path );
>
>    /* Replace "/dev/" with "/dev/r" */

The function is supposed to replace -on most BSDs- access to a block device to the equivalent raw device,
returning in a string the correct device name to use.
Now, it's true that in the current code the comment reads one thing and the code does the exact opposite, but
both the original code and the patched one don't make sense to me:
-if the string starts with /dev/ it's not necesserily correct or wrong
-it the string starts with /dev/r it IS correct and doesn't need to be replaced, so
return (char *) strdup( path );
is fine

In practice to me the right code to use seems:
   /* If the string starts with "/dev/r"  then return it */
   if(!strncmp( path, "/dev/r", 6 ) )
     return (char *) strdup( path );


of course if the string doesn't start with /dev/ it's wrong, but this is the idiot's business (that didn't specify a correct
device to use).
Am I missing something?