[sane-devel] [patch] fix SANE_Device

Oliver Rauch Oliver.Rauch@Rauch-Domain.DE
18 Nov 2004 14:18:14 +0100


Am Don, 2004-11-18 um 06.09 schrieb Frank Zago:
> Oliver Rauch wrote:
> > Am Mit, 2004-11-17 um 20.58 schrieb fzago@austin.rr.com:
> > 
> >>>Hello Frank.
> >>>
> >>>I am not a fan of this patch.
> >>>I think it will work but I think it is a hack.
> >>>
> >>>It seems that only a few backends have problems with this.
> >>
> >>A few backend are creating the name of the manufacturer from the scsi inquiry.
> >>A lot of backends are creating their mode list from the device.
> >>This patch will allow us to get rid of about 150 (IMO legitimate) warnings. The current code is not clean.
> >>
> >>
> >>
> >>>Why not fix it in a proper way:
> >>
> >>The non hack solution is to have two headers: one for the symbols exported and one for internal use. Same as what is done with the linux kernel structure s.
> >>
> >>
> >>>create the strings as non const and then set the
> >>>sane_device structur to point to these strings?!
> >>
> >>This will not work. That why there is so much warnings.
> > 
> > 
> > IMO thats not ture.
> > It is allowed to do this:
> > 
> > 	char *hello;
> > 	const char *hello_const;
> > 
> >  	  hello="ABC";
> > 
> > 	  hello_const = hello;
> > 
> > 
> > 
> > and this produces a warning:
> > 
> > 	  *hello_const = 'A';
> > 
> > what is correct becaus we use a pointer to a const char to change the
> > char so it is not const any more.
> > 
> > It is not a const pointer to a char, it is a pointer to a const char.
> > And it is allowed to make a pointer of type pointer to const char point
> > to a (non const) char.
> > 
> > I attach a little test c-code for this. Compile with
> > 
> > gcc consttest.c -o consttest -Wall
> > 
> > (gcc version 3.2.2 20030222)
> > 
> > Oliver
> > 
> > 
> >>>But when I am the only one who is against this patch
> >>>then I will be quiet and it will be ok for me. It is nothing
> >>>that will steal my sleep :)
> >>>
> >>
> >>Your input is appreciated. Thanks.
> >>
> >>Frank.
> >>
> >>
> >>
> >>
> >>
> >>
> >>------------------------------------------------------------------------
> >>
> >>#include "stdio.h"
> >>#include "stdlib.h"
> >>
> >>int main()
> >>{
> >> const char *hello_const;
> >> char *hello;
> >>
> >>  printf("defining hello=\"Hello\"\n");
> >>  printf("defining hello_const = hello\n");
> >>  hello = "Hello";
> >>  hello_const = hello;
> >>  printf("hello = %s\n", hello);
> >>  printf("hello_const = %s\n", hello_const);
> >>  printf("\n");
> >>
> >>  printf("defining hello = (char *) malloc(10)\n");
> >>  printf("defining *hello = \'A\'\n");
> >>  printf("defining *(hello+1) = \'B\'\n");
> >>  printf("defining hello_const = hello\n");
> >>  hello= (char *) malloc(10);
> >>  hello_const = hello;
> >>  *hello='A';
> >>  *(hello+1)='B';
> >>  hello[2]=0;
> >>  printf("hello = %s\n", hello);
> >>  printf("hello_const = %s\n", hello_const);
> >>  printf("\n");
> >>
> >>  printf("unallowed defining *hello_const = \'X\'\n");
> >>  *hello_const='X';
> >>
> >>  printf("hello = %s\n", hello);
> >>  printf("hello_const = %s\n", hello_const);
> >>
> >> return 0;
> >>}
> 
> Your example is missing the one thing I'd like to see fixed. How do you free 
> your memory allocated?
> free(hello_const) will generate a warning.
> free((void *)hello_const) will also generate a warning.
> 
> So do we fix:
> 
> dev->sane.name = strdup (devname);
> ...
> free(dev->sane.name);
> 
> 
> with:
> 
> unconst_name = strdup (devname);
> dev->sane.name = unconst_name;
> ..
> dev->sane.name = NULL;
> free(unconst_name);

Thats what I prefer.

We only have to keep the original pointer
and everything is ok.

Oliver