[libdvdcss-devel] try again: patch for negative caching of keys

[Diego Biurrun]

On Sun, Aug 28, 2011 at 12:23:17PM +0200, Marc Espie wrote:
> I sent this a few weeks ago.  I haven't gotten any useful feedback from it.
> (I'm french, if you really have trouble expressing yourself in english).

I had email troubles in between, but I did not see any previous emails
from you.

> I really would like to see negative caching in dvdcss.

What is negative caching?

> I believe storing the PACKAGE_VERSION is the simplest thing to do, in case
> someone figures out a bug that means key cracking was actually incorrect
> later.

Hmmm...

> I'm willing to spend a bit more time working on that patch, but GIVE ME
> CONCRETE DIRECTIONS, not some fuzzy things like "I would do that differently"
> (with about NO INDICATION of what should be done differently) or "this
> should be separate patches" (I'll make a wild guess at: configure.ac separated
> from the css code proper).

I'm not a libdvdcss developer, but here are some comments anyway..

> --- configure.ac.orig	Fri Aug 29 20:59:00 2008
> +++ configure.ac	Wed Aug 17 11:51:22 2011
> @@ -1,4 +1,4 @@
> -AC_INIT(src/libdvdcss.c)
> +AC_INIT(libdvdcss, 1.2.10)

The AC_INIT syntax changed between autoconf versions, you are using the
newer one.  I'm not sure what the policy for supporting old autotools
versions is for libdvdcss.

> --- src/css.c.orig	Fri Aug 29 20:42:47 2008
> +++ src/css.c	Wed Aug 17 11:52:49 2011
> @@ -172,10 +172,13 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
>          {
>              char psz_key[KEY_SIZE * 3];
>              unsigned int k0, k1, k2, k3, k4;
> +	    int n;
>  
> -            psz_key[KEY_SIZE * 3 - 1] = '\0';
> +	    memset(psz_key, 0, sizeof psz_key);
> +	    n = read( i_fd, psz_key, KEY_SIZE * 3 - 1 );
> +            close( i_fd );
>  
> -            if( read( i_fd, psz_key, KEY_SIZE * 3 - 1 ) == KEY_SIZE * 3 - 1
> +            if( n == KEY_SIZE * 3 - 1

You add tabs to a file with only spaces used for indentation, same below.

> @@ -189,9 +192,15 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
>                  /* Don't try to save it again */
>                  b_cache = 0;
>                  i_ret = 1;
> -            }
> +            } 

trailing whitespace

> +#if defined PACKAGE_VERSION
> +	    else if (n > 0 && strcmp(psz_key, PACKAGE_VERSION "\n") == 0) 
> +	    {
> +	    	/* didn't crack it, negative caching */
> +		return -1;
> +	    }
> +#endif

What you want to achieve with the #ifdef is a mystery to me.
PACKAGE_VERSION should always be set after your configure.ac change,
so the condition will always be true and thus superfluous.

Diego