[Debburn-devel] [Debburn-changes] r787 - in cdrkit/trunk: genisoimage genisoimage/diag icedax include libhfs_iso librols libusal libusal/usal wodim

[Peter Samuelson]

[harald-hoyer-guest at alioth.debian.org]
> --- cdrkit/trunk/genisoimage/diag/dump.c	2008-01-30 20:50:51 UTC (rev 786)
> +++ cdrkit/trunk/genisoimage/diag/dump.c	2008-02-25 11:14:07 UTC (rev 787)
> @@ -366,9 +366,10 @@
>  	on_comerr((void(*)(int, void *))reset_tty, NULL);
>  
>  	do {
> +	        int ret;
>  		if (file_addr < (off_t)0) file_addr = (off_t)0;
>  		showblock(1);
> -		read(STDIN_FILENO, &c, 1);
> +		ret = read(STDIN_FILENO, &c, 1);
>  		if (c == 'a')
>  			file_addr -= PAGE;
>  		if (c == 'b')

I _really_ don't like this.  If we need to tell gcc not to warn us
about lack of error handling, we should use (void) rather than making
the code _look_ like we are handling errors when in reality we are not.
And please add a comment saying why error checking doesn't matter here.

(Also, read() returns ssize_t, not int.)

Same goes for lots of other instances of the same thing.

> --- cdrkit/trunk/genisoimage/jte.c	2008-01-30 20:50:51 UTC (rev 786)
> +++ cdrkit/trunk/genisoimage/jte.c	2008-02-25 11:14:07 UTC (rev 787)
> @@ -535,13 +535,13 @@
>      }
>  
>      memset(buf, 0, sizeof(buf));
> -    while (fgets(buf, sizeof(buf), md5_file))
> +    while (fgets((char *)buf, sizeof(buf), md5_file))

This sort of thing just makes me wonder: why is 'buf' not already
defined as a char* or void* ?  Shouldn't that be fixed instead?

> --- cdrkit/trunk/librols/fexec.c	2008-01-30 20:50:51 UTC (rev 786)
> +++ cdrkit/trunk/librols/fexec.c	2008-02-25 11:14:07 UTC (rev 787)
> @@ -204,6 +204,8 @@
>  	int	o[3];
>  	int	f[3];
>  	int	errsav;
> +
> +	o[0] = o[1] = o[2] = f[0] = f[1] = f[2] = 0;

int o[] = { 0, 0, 0 }, f[] = { 0, 0, 0 };    seems simpler.


> --- cdrkit/trunk/librols/raisecond.c	2008-01-30 20:50:51 UTC (rev 786)
> +++ cdrkit/trunk/librols/raisecond.c	2008-02-25 11:14:07 UTC (rev 787)
> @@ -58,8 +58,8 @@
>  #ifndef	STDERR_FILENO
>  #define	STDERR_FILENO	2
>  #endif
> -#define	eprints(a)	(void)write(STDERR_FILENO, (a), sizeof (a)-1)
> -#define	eprintl(a)	(void)write(STDERR_FILENO, (a), strlen(a))
> +#define	eprints(a)	do { int ret; ret = write(STDERR_FILENO, (a), sizeof (a)-1); } while (0)
> +#define	eprintl(a)	do { int ret; ret = write(STDERR_FILENO, (a), strlen(a)); } while (0)

What is the purpose of this change?

> --- cdrkit/trunk/libusal/scsi-linux-sg.c	2008-01-30 20:50:51 UTC (rev 786)
> +++ cdrkit/trunk/libusal/scsi-linux-sg.c	2008-02-25 11:14:07 UTC (rev 787)
> @@ -181,7 +181,7 @@
>  struct usal_local {
>  	int	usalfile;		/* Used for SG_GET_BUFSIZE ioctl()*/
>  	short	usalfiles[MAX_SCG][MAX_TGT][MAX_LUN];
> -  char *filenames[MAX_SCG][MAX_TGT][MAX_LUN];
> +        char    *filenames[MAX_SCG][MAX_TGT][MAX_LUN];

What is the purpose of this change?

> @@ -594,13 +596,14 @@
>  
>  			/* that's crap, should not be reached in non-scan mode.
>  			 * Let's see whether it can be mapped to an atapi
> -			 * device to emulate some old cludge's behaviour. */
> +			 * device to emulate some old cludge's behaviour. 
>  			if(!in_scanmode && busno < 1000 && busno >=0) {
>  				fake_atabus=1;
>  				fprintf(stderr, "Unable to open this SCSI ID. Trying to map to old ATA syntax."
>  						"This workaround will disappear in the near future. Fix your configuration.");
>  				goto retry_scan_open;
>  			}
> +			*/

This doesn't look like it has anything to do with gcc warnings.

> @@ -801,7 +804,7 @@
>  	for (i = 0; i < 1000; i++) {	/* Read at least 32k from /dev/sg* */
>  		int	ret;
>  
> -		ret = read(f, &sg_rep, sizeof (sg_rep));
> +		ret = read(f, &sg_rep, sizeof (struct sg_rep));

What is the purpose of this change?  sizeof(variable) is generally
better than sizeof(type), because it stays correct if the type changes.

> --- cdrkit/trunk/libusal/scsierrs.c	2008-01-30 20:50:51 UTC (rev 786)
> +++ cdrkit/trunk/libusal/scsierrs.c	2008-02-25 11:14:07 UTC (rev 787)
> @@ -55,6 +55,7 @@
>  const char	*usal_sensemsg(int, int, int, const char **, char *, int maxcnt);
>  int usal__errmsg(SCSI *usalp, char *obuf, int maxcnt, struct scsi_sense *, 
>  					 struct scsi_status *, int);
> +#ifdef ABC
>  /*
>   * Map old non extended sense to sense key.
>   */
> @@ -67,6 +68,7 @@
>  	6, 6, 6, 5,  4,11,11,11			/* 0x28-0x2f */
>  };
>  #define	MAX_ADAPTEC_KEYS (sizeof (sd_adaptec_keys))
> +#endif

The more usual way to comment out a large block of code is "#if 0"
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/