Re: [KJ] [PATCH] Reduction of compile warnings (Warning: long post

[Jim Nelson <james4765@cwazy.co.uk>]

[-- Attachment #1: Type: text/plain, Size: 7249 bytes --]

>>Nope.  Got the swsusp-nocompile.txt, but everything else was munched by the 
>>mailing list.
> 
> 
> Jim,
> 
> Could you double-check, please?  I got a copy of my post back from the 
> mailing list and it is copacetic -- I can read and extract everything; 3 
> "Text/Plain" attachments with 3 "attachment information" parts put in by 
> Pegasus.
> 

My mistake.  I'm not used to seeing 6 attachments... </foot-in-mouth>

> 
>>You'll definitely want to split the patch up by driver, or directory at the very 
>>least.  You'll also want to send the patches as inlined text, with the 
>>Signed-off-by: line (see Documentation/SubmittingPatches and 
>>http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt for more pointers).
> 
> 
> Ok, I see that I missed a couple of things... the problem with my patch 
> is that it is very many one or two line changes in many files.  I count, 
> using:
> grep "^diff" linux-2.6.11-rc3-mm_compile-warnings.patch  | wc -l
> 49 different files... that is a bit much to break up, no?  What do I do 
> about this?
> 

If there's changes to a .c and .h, or changes to a set of files that relate to one 
driver (drivers/char/ip2, for example), they can come in one patch, but otherwise, 
one driver per patch - even if it is a lot of patches.
> 
>>There's a few good scripts out there for sending groups of patches to mailing 
>>lists.  I've also hacked up something to chop up a large diff by file, but I'm 
>>almost embarassed to show it in public :)
> 
> 
> Can you provide links to these scripts?
> 

http://www.speakeasy.net/~pj99/sgi/sendpatchset

> 
>>Jim
>>
> 

A few things I noticed about the patch you sent:

> diff -Nurdp linux-2.6.11-rc3-mm-original/.config linux-2.6.11-rc3-mm/.config
> --- linux-2.6.11-rc3-mm-original/.config	2005-02-07 16:34:23.000000000 +0200
> +++ linux-2.6.11-rc3-mm/.config	2005-02-08 18:47:46.954507280 +0200
> @@ -1,7 +1,7 @@
>  #
>  # Automatically generated make config: don't edit
>  # Linux kernel version: 2.6.11-rc3-mm1
> -# Mon Feb  7 16:34:23 2005
> +# Tue Feb  8 18:47:46 2005
>  #
>  CONFIG_X86=y
>  CONFIG_MMU=y
> 

> diff -Nurdp linux-2.6.11-rc3-mm-original/include/linux/autoconf.h linux-2.6.11-rc3-mm/include/linux/autoconf.h
> --- linux-2.6.11-rc3-mm-original/include/linux/autoconf.h	2005-02-07 16:34:23.000000000 +0200
> +++ linux-2.6.11-rc3-mm/include/linux/autoconf.h	2005-02-08 18:47:46.954507280 +0200
> @@ -1,7 +1,7 @@
>  /*
>   * Automatically generated C config: don't edit
>   * Linux kernel version: 2.6.11-rc3-mm1
> - * Mon Feb  7 16:34:23 2005
> + * Tue Feb  8 18:47:46 2005
>   */
>  #define AUTOCONF_INCLUDED
>  #define CONFIG_X86 1

Add to your diff command the '-X dontdiff-osdl', using the file 
http://developer.osdl.org/rddunlap/scripts/dontdiff-osdl


> diff -Nurdp linux-2.6.11-rc3-mm-original/drivers/scsi/aha1740.c linux-2.6.11-rc3-mm/drivers/scsi/aha1740.c
> --- linux-2.6.11-rc3-mm-original/drivers/scsi/aha1740.c	2005-02-03 03:57:16.000000000 +0200
> +++ linux-2.6.11-rc3-mm/drivers/scsi/aha1740.c	2005-02-08 18:22:51.000000000 +0200
> @@ -588,6 +588,7 @@ static Scsi_Host_Template aha1740_templa
>  static int aha1740_probe (struct device *dev)
>  {
>  	int slotbase;
> +	int err_retval = -ENODEV;
>  	unsigned int irq_level, irq_type, translation;
>  	struct Scsi_Host *shpnt;
>  	struct aha1740_hostdata *host;
> @@ -642,7 +643,10 @@ static int aha1740_probe (struct device 
>  	}
>  
>  	eisa_set_drvdata (edev, shpnt);
> -	scsi_add_host (shpnt, dev); /* XXX handle failure */
> +	if (scsi_add_host (shpnt, dev)) {
> +		err_retval = -EIO;
> +		goto err_unmap; /* XXX handle failure */
> +	}
>  	scsi_scan_host (shpnt);
>  	return 0;
>  
> @@ -654,7 +658,7 @@ static int aha1740_probe (struct device 
>   err_release_region:
>  	release_region(slotbase, SLOTSIZE);
>  
> -	return -ENODEV;
> +	return err_retval;
>  }
>  

> diff -Nurdp linux-2.6.11-rc3-mm-original/drivers/scsi/aic7xxx/aic79xx_osm.c linux-2.6.11-rc3-mm/drivers/scsi/aic7xxx/aic79xx_osm.c
> --- linux-2.6.11-rc3-mm-original/drivers/scsi/aic7xxx/aic79xx_osm.c	2005-02-03 03:55:53.000000000 +0200
> +++ linux-2.6.11-rc3-mm/drivers/scsi/aic7xxx/aic79xx_osm.c	2005-02-07 15:49:09.000000000 +0200
> @@ -2065,7 +2065,8 @@ ahd_linux_register_host(struct ahd_softc
>  	ahd_unlock(ahd, &s);
>  
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,0)
> -	scsi_add_host(host, &ahd->dev_softc->dev); /* XXX handle failure */
> +	if(scsi_add_host(host, &ahd->dev_softc->dev))
> +		return (EIO); /* XXX handle failure */
>  	scsi_scan_host(host);
>  #endif
>  	return (0);

> diff -Nurdp linux-2.6.11-rc3-mm-original/drivers/scsi/aic7xxx/aic7xxx_osm.c linux-2.6.11-rc3-mm/drivers/scsi/aic7xxx/aic7xxx_osm.c
> --- linux-2.6.11-rc3-mm-original/drivers/scsi/aic7xxx/aic7xxx_osm.c	2005-02-03 03:55:07.000000000 +0200
> +++ linux-2.6.11-rc3-mm/drivers/scsi/aic7xxx/aic7xxx_osm.c	2005-02-07 15:54:42.000000000 +0200
> @@ -1729,7 +1729,8 @@ ahc_linux_register_host(struct ahc_softc
>  	ahc_unlock(ahc, &s);
>  
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,0)
> -	scsi_add_host(host, (ahc->dev_softc ? &ahc->dev_softc->dev : NULL)); /* XXX handle failure */
> +	if(scsi_add_host(host, (ahc->dev_softc ? &ahc->dev_softc->dev : NULL)))
> +		return (EIO); /* XXX handle failure */
>  	scsi_scan_host(host);
>  #endif
>  	return (0);

> diff -Nurdp linux-2.6.11-rc3-mm-original/drivers/usb/image/microtek.c linux-2.6.11-rc3-mm/drivers/usb/image/microtek.c
> --- linux-2.6.11-rc3-mm-original/drivers/usb/image/microtek.c	2005-02-03 03:57:04.000000000 +0200
> +++ linux-2.6.11-rc3-mm/drivers/usb/image/microtek.c	2005-02-07 21:18:47.000000000 +0200
> @@ -809,7 +809,11 @@ static int mts_usb_probe(struct usb_inte
>  		goto out_free_urb;
>  
>  	new_desc->host->hostdata[0] = (unsigned long)new_desc;
> -	scsi_add_host(new_desc->host, NULL); /* XXX handle failure */
> +	if(scsi_add_host(new_desc->host, NULL)) { /* XXX handle failure */
> +		usb_free_urb(new_desc->urb);
> +		kfree(new_desc);
> +		return -EIO;
> +	}
>  	scsi_scan_host(new_desc->host);
>  
>  	usb_set_intfdata(intf, new_desc);

Since you're fixing the failure handling, you can also remove the XXX comments.

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,0) is not needed for drivers that are 
in the mainline kernel - only needed for drivers that are compiled against 
multiple versions of the kernel.

> diff -Nurdp linux-2.6.11-rc3-mm-original/sound/oss/emu10k1/cardwi.c linux-2.6.11-rc3-mm/sound/oss/emu10k1/cardwi.c
> --- linux-2.6.11-rc3-mm-original/sound/oss/emu10k1/cardwi.c	2005-02-03 03:56:11.000000000 +0200
> +++ linux-2.6.11-rc3-mm/sound/oss/emu10k1/cardwi.c	2005-02-08 16:21:22.000000000 +0200
> @@ -306,8 +306,10 @@ void emu10k1_wavein_getxfersize(struct w
>  
>  static void copy_block(u8 __user *dst, u8 * src, u32 str, u32 len, u8 cov)
>  {
> -	if (cov == 1)
> -		__copy_to_user(dst, src + str, len);
> +	if (cov == 1) {
> +		if(__copy_to_user(dst, src + str, len))
> +			BUG();
> +	}
>  	else {
>  		u8 byte;
>  		u32 i;
> @@ -316,7 +318,8 @@ static void copy_block(u8 __user *dst, u
>  
>  		for (i = 0; i < len; i++) {
>  			byte = src[2 * i] ^ 0x80;
> -			__copy_to_user(dst + i, &byte, 1);
> +			if(__copy_to_user(dst + i, &byte, 1))
> +				BUG();
>  		}
>  	}
>  }



A little dramatic, don't you think?

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors