[MPlayer-cvslog] r23727 - trunk/libdvdcss/libdvdcss.c

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Tue Jul 17 00:37:30 CEST 2007


Hello,
On Mon, Jul 16, 2007 at 11:24:17PM +0200, Diego Biurrun wrote:
> On Sat, Jul 14, 2007 at 03:23:46PM +0200, Reimar Döffinger wrote:
> > On Sat, Jul 14, 2007 at 02:50:00PM +0200, Diego Biurrun wrote:
> > > On Sat, Jul 14, 2007 at 10:06:31AM +0200, Reimar Döffinger wrote:
> > > > It is not a very relevant change though (esp. compared to  r23728).
> > > > You could also cast all parameters to unsigned instead.
> > > > IMO using sprintf (and even snprintf) here is a stupid idea anyway.
> > > 
> > > I'm asking because I wish to keep our diffs to upstream as small as
> > > possible.  If r23728 is enough to solve the problem then this might as
> > > well be reverted.
> > 
> > It solves the critical problem, but without it the format strings do not
> > match the type, though I do not know on what type of system it will make
> > a difference.
> 
> Umm, you mean Ivan's commit solves the critical problem while yours makes
> the types match?
> 
> My point is that if your commit is more or less cosmetic then I would
> suggest reverting it to minimize the diff towards upstream.

AFAICT sprintf behaviour is unspecified when the types mismatch as is
the case without this patch.
But actually this is moot, since AFAICT sprintf isn't really specified
sufficiently to be used in this way if you are pedantic. Which is why I
proposed a different patch on libdvdcss-devel - unfortunately the mails
never got through, nor did I receive any of the mails sent, despite I
received a confirmation mail that I was subscribed.
Is there a chance of getting dvdcss upstream that is not just a joke (or
more precisely a black hole)?
I'll attach the mails I sent, since you found a magic way to get through
the black hole they use as email filtering.

Greetings,
Reimar Döffinger
-------------- next part --------------
Date: Sat, 14 Jul 2007 16:26:59 +0200
From: Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>
To: libdvdcss-devel at videolan.org
Subject: [RFC] some cosmetic changes (pointless {} and extern)
Message-ID: <20070714142659.GB28740 at 1und1.de>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="k1lZvvs/B4yU6o8G"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
User-Agent: Mutt/1.5.15 (2007-04-06)
Status: RO


--k1lZvvs/B4yU6o8G
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Hello,
possibly this is intentional, then sorry for the noise, but attached
patch (libdvdcss.c only so far) would remove {} for single-statement
blocks which IMO only clutter the code.
It would also remove "LIBDVDCSS_EXPORT char * dvdcss_interface_2;" since
the exact same line is already in dvdcss.h which is included.

Greetings,
Reimar Döffinger

--k1lZvvs/B4yU6o8G
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: attachment; filename="dvdcsscosm.diff"

diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index c07c1b6..152148c 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -142,7 +142,6 @@
  * The variable itself contains the exact version number of the library,
  * which can be useful for specific feature needs.
  */
-LIBDVDCSS_EXPORT char * dvdcss_interface_2;
 char * dvdcss_interface_2 = VERSION;
 
 /**
@@ -178,9 +177,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
      */
     dvdcss = malloc( sizeof( struct dvdcss_s ) );
     if( dvdcss == NULL )
-    {
         return NULL;
-    }
 
     /*
      *  Initialize structure with default values
@@ -213,17 +210,11 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
     if( psz_method != NULL )
     {
         if( !strncmp( psz_method, "key", 4 ) )
-        {
             dvdcss->i_method = DVDCSS_METHOD_KEY;
-        }
         else if( !strncmp( psz_method, "disc", 5 ) )
-        {
             dvdcss->i_method = DVDCSS_METHOD_DISC;
-        }
         else if( !strncmp( psz_method, "title", 5 ) )
-        {
             dvdcss->i_method = DVDCSS_METHOD_TITLE;
-        }
         else
         {
             print_error( dvdcss, "unknown decrypt method, please choose "
@@ -263,13 +254,9 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
                 /* Get the "Application Data" folder for the current user */
                 if( p_getpath( NULL, CSIDL_APPDATA | CSIDL_FLAG_CREATE,
                                NULL, SHGFP_TYPE_CURRENT, psz_home ) == S_OK )
-                {
                     FreeLibrary( p_dll );
-                }
                 else
-                {
                     *psz_home = '\0';
-                }
             }
             FreeLibrary( p_dll );
         }
@@ -290,19 +277,13 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
         /* Try looking in password file for home dir. */
         p_pwd = getpwuid(getuid());
         if( p_pwd )
-        {
             psz_home = p_pwd->pw_dir;
-        }
 #   endif
 
         if( psz_home == NULL )
-        {
             psz_home = getenv( "HOME" );
-        }
         if( psz_home == NULL )
-        {
             psz_home = getenv( "USERPROFILE" );
-        }
 
         /* Cache our keys in ${HOME}/.dvdcss/ */
         if( psz_home )
@@ -320,9 +301,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
     if( psz_cache != NULL )
     {
         if( psz_cache[0] == '\0' || !strcmp( psz_cache, "off" ) )
-        {
             psz_cache = NULL;
-        }
         /* Check that we can add the ID directory and the block filename */
         else if( strlen( psz_cache ) + 1 + 32 + 1 + (KEY_SIZE * 2) + 10 + 1
                   > PATH_MAX )
@@ -412,21 +391,15 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
 
         i_ret = dvdcss->pf_seek( dvdcss, 0 );
         if( i_ret != 0 )
-        {
             goto nocache;
-        }
 
         i_ret = dvdcss->pf_read( dvdcss, p_sector, 1 );
         if( i_ret != 1 )
-        {
             goto nocache;
-        }
 
         if( p_sector[0] == 0x00 && p_sector[1] == 0x00
              && p_sector[2] == 0x01 && p_sector[3] == 0xba )
-        {
             goto nocache;
-        }
 
         /* The data we are looking for is at sector 16 (32768 bytes):
          *  - offset 40: disc title (32 uppercase chars)
@@ -434,15 +407,11 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
 
         i_ret = dvdcss->pf_seek( dvdcss, 16 );
         if( i_ret != 16 )
-        {
             goto nocache;
-        }
 
         i_ret = dvdcss->pf_read( dvdcss, p_sector, 1 );
         if( i_ret != 1 )
-        {
             goto nocache;
-        }
 
         /* Get the disc title */
         psz_title = (char *)p_sector + 40;
@@ -456,9 +425,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
                 break;
             }
             else if( psz_title[i] == '/' || psz_title[i] == '\\' )
-            {
                 psz_title[i] = '-';
-            }
         }
 
         /* Get the date + serial */
@@ -487,9 +454,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
         {
              psz_key[0] = '-';
              for( i = 0; i < KEY_SIZE; i++ )
-             {
                  sprintf( &psz_key[1+i*2], "%.2x", dvdcss->css.p_disc_key[i] );
-             }
              psz_key[1 + KEY_SIZE * 2] = '\0';
         }
         else
@@ -537,9 +502,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
 
 #ifndef WIN32
     if( psz_raw_device != NULL )
-    {
         _dvdcss_raw_open( dvdcss, psz_raw_device );
-    }
 #endif
 
     /* Seek at the beginning, just for safety. */
@@ -597,9 +560,7 @@ LIBDVDCSS_EXPORT int dvdcss_seek ( dvdcss_t dvdcss, int i_blocks, int i_flags )
     {
         /* check the title key */
         if( _dvdcss_title( dvdcss, i_blocks ) )
-        {
             return -1;
-        }
     }
 
     return dvdcss->pf_seek( dvdcss, i_blocks );
@@ -639,9 +600,7 @@ LIBDVDCSS_EXPORT int dvdcss_read ( dvdcss_t dvdcss, void *p_buffer,
     if( i_ret <= 0
          || !dvdcss->b_scrambled
          || !(i_flags & DVDCSS_READ_DECRYPT) )
-    {
         return i_ret;
-    }
 
     if( ! memcmp( dvdcss->css.p_title_key, "\0\0\0\0\0", 5 ) )
     {
@@ -715,9 +674,7 @@ LIBDVDCSS_EXPORT int dvdcss_readv ( dvdcss_t dvdcss, void *p_iovec,
     if( i_ret <= 0
          || !dvdcss->b_scrambled
          || !(i_flags & DVDCSS_READ_DECRYPT) )
-    {
         return i_ret;
-    }
 
     /* Initialize loop for decryption */
     iov_base = _p_iovec->iov_base;
@@ -728,9 +685,7 @@ LIBDVDCSS_EXPORT int dvdcss_readv ( dvdcss_t dvdcss, void *p_iovec,
     {
         /* Check that iov_len is a multiple of 2048 */
         if( iov_len & 0x7ff )
-        {
             return -1;
-        }
 
         while( iov_len == 0 )
         {
@@ -776,9 +731,7 @@ LIBDVDCSS_EXPORT int dvdcss_close ( dvdcss_t dvdcss )
     i_ret = _dvdcss_close( dvdcss );
 
     if( i_ret < 0 )
-    {
         return i_ret;
-    }
 
     free( dvdcss->psz_device );
     free( dvdcss );

--k1lZvvs/B4yU6o8G--
-------------- next part --------------
Date: Sat, 14 Jul 2007 16:27:08 +0200
From: Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>
To: libdvdcss-devel at videolan.org
Subject: [PATCH] do not use sprintf for conversion to hexadecimal
Message-ID: <20070714142708.GC28740 at 1und1.de>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="aVD9QWMuhilNxW9f"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
User-Agent: Mutt/1.5.15 (2007-04-06)
Status: RO


--aVD9QWMuhilNxW9f
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Hello,
at least my sprintf man page is not too specific as to what exactly the
%x specifier will output, so I think it is a good idea to not use it at
all IMO for converting the key, which attached patch would do.
At the very least, the psz_serial arguments must be cast to unsigned
int, because 1) %x is for int, not for char, so the type does not match
the format string 2) like it is now, it will be converted to the larger
type with sign extensions and sprintf will print 8 characters instead
of two (e.g. "fffffffe" instead of "fe", see also the patch Diego
Biurrun sent while I was preparing this *g*).

Greetings,
Reimar Döffinger

--aVD9QWMuhilNxW9f
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: attachment; filename="tohex.diff"

diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index c07c1b6..6f1c55a 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -145,6 +145,24 @@
 LIBDVDCSS_EXPORT char * dvdcss_interface_2;
 char * dvdcss_interface_2 = VERSION;
 
+static void put_hex4(uint8_t *dst, uint8_t n) {
+  n &= 0x0f;
+  if (n > 9) n += 'a' - 10;
+  else n += '0';
+  *dst = n;
+}
+
+static void to_hex(void *dst, void *src, int len) {
+  uint8_t d = dst;
+  uint8_t s = src;
+  int i;
+  for (i = 0; i < len; i++) {
+    put_hex4(d++, *s >> 4);
+    put_hex4(d++, *s & 0x0f);
+    s++;
+  }
+}
+
 /**
  * \brief Open a DVD device or directory and return a dvdcss instance.
  *
@@ -470,13 +488,9 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
         {
             if( psz_serial[i] < '0' || psz_serial[i] > '9' )
             {
-                char psz_tmp[16 + 1];
-                sprintf( psz_tmp,
-                         "%.2x%.2x%.2x%.2x%.2x%.2x%.2x%.2x",
-                         psz_serial[0], psz_serial[1], psz_serial[2],
-                         psz_serial[3], psz_serial[4], psz_serial[5],
-                         psz_serial[6], psz_serial[7] );
-                memcpy( psz_serial, psz_tmp, 16 );
+                uint8_t psz_tmp[8];
+                memcpy(psz_tmp, psz_serial, 8);
+                to_hex(psz_serial, psz_serial, 8);
                 break;
             }
         }
@@ -486,10 +500,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
         if( dvdcss->b_scrambled )
         {
              psz_key[0] = '-';
-             for( i = 0; i < KEY_SIZE; i++ )
-             {
-                 sprintf( &psz_key[1+i*2], "%.2x", dvdcss->css.p_disc_key[i] );
-             }
+             to_hex(psz_key + 1, dvdcss->css.p_disc_key, KEY_SIZE);
              psz_key[1 + KEY_SIZE * 2] = '\0';
         }
         else

--aVD9QWMuhilNxW9f--


More information about the MPlayer-cvslog mailing list