On 9 Feb 2005 at 5:43, Jim Nelson wrote: > >>Nope. Got the swsusp-nocompile.txt, but everything else was munched by the > >>mailing list. > > > > > > Jim, Jim, Thanks very much for the helpful input. The fixed patches will follow shortly. Please read on for my further comments and questions. > > > > 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... No problem ;-) I know that I am still learning how to do this; plus I am having a problem trying to figure out, if possible, how to stop Pegasus from attaching a second file with "attachment information" if I am sending text/plain. I guess I'll just do it inline as below. > > > > >>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. Ok, I'll do this... I see the reasons for doing this even though it is a pain! > > > >>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 Unfortunately this won't work for me until I finally move my email completely over to Linux. Yes, I know that I am making things harder on myself, but that's what's happening right now. > > > > >>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 Ok, I'll do that. I thought I had removed those files, but the dontdiff is a better solution than doing it by hand, anyway. > > > > 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. Ok, done and done. > > #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. I don't understand what you mean by this comment. Do you mean that I should remove the #if/#endif statements? Aren't they in the original and I should leave them? What should I do here? > > > 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? Perhaps, but perhaps not. What does it mean when copy_to/from_user fails?? Doesn't it means that something is really messed up and it, most usually, is pretty much a panic situation, no? Also, in this particular file, BUG() is used for things like invalid data situations... just staying consistent. What do you suggest here? >