| Message ID | 1277879827-18233-1-git-send-email-justinmattock@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello, On 06/30/2010 08:37 AM, Justin P. Mattock wrote: > The below patch fixes a warning message during compiling of the kernel > CC drivers/ata/ahci.o > drivers/ata/ahci.c: In function 'ahci_init_one': > drivers/ata/ahci.c:1045:2: warning: comparison between 'enum <anonymous>' and 'enum <anonymous>' > > Signed-off-by: Justin P. Mattock <justinmattock@gmail.com> Hmm, is this something we wanna go around the kernel and updating sources? Or should we just flick a gcc option? > @@ -1038,11 +1038,11 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > struct device *dev = &pdev->dev; > struct ahci_host_priv *hpriv; > struct ata_host *host; > - int n_ports, i, rc; > + int n_ports, i, rc, sum = AHCI_MAX_CMDS; > > VPRINTK("ENTER\n"); > > - WARN_ON(ATA_MAX_QUEUE > AHCI_MAX_CMDS); > + WARN_ON(ATA_MAX_QUEUE > sum); And, just do WARN_ON((int)ATA_MAX_QUEUE > (int)AHCI_MAX_CMDS) Thanks.
On 06/29/2010 11:49 PM, Tejun Heo wrote: > Hello, > > On 06/30/2010 08:37 AM, Justin P. Mattock wrote: >> The below patch fixes a warning message during compiling of the kernel >> CC drivers/ata/ahci.o >> drivers/ata/ahci.c: In function 'ahci_init_one': >> drivers/ata/ahci.c:1045:2: warning: comparison between 'enum<anonymous>' and 'enum<anonymous>' >> >> Signed-off-by: Justin P. Mattock<justinmattock@gmail.com> > > Hmm, is this something we wanna go around the kernel and updating > sources? Or should we just flick a gcc option? > the flick a gcc option seems easier todo, but my guess its probably not the right way of dealing with the issue(even a warning). >> @@ -1038,11 +1038,11 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) >> struct device *dev =&pdev->dev; >> struct ahci_host_priv *hpriv; >> struct ata_host *host; >> - int n_ports, i, rc; >> + int n_ports, i, rc, sum = AHCI_MAX_CMDS; >> >> VPRINTK("ENTER\n"); >> >> - WARN_ON(ATA_MAX_QUEUE> AHCI_MAX_CMDS); >> + WARN_ON(ATA_MAX_QUEUE> sum); > > And, just do WARN_ON((int)ATA_MAX_QUEUE> (int)AHCI_MAX_CMDS) > > Thanks. > that builds clean. keep in mind I just compile tested, no rebooting or anything. Should I just resend with what you posted, and call it that, or is this something that needs more? Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/30/2010 09:06 AM, Justin P. Mattock wrote: >> Hmm, is this something we wanna go around the kernel and updating >> sources? Or should we just flick a gcc option? > > the flick a gcc option seems easier todo, but my guess its probably not > the right way of dealing with the issue(even a warning). What does the extra warning buy us? There are several places which use anonymous enums for constants and I can't see what the benefit of this warning would be. >> And, just do WARN_ON((int)ATA_MAX_QUEUE > (int)AHCI_MAX_CMDS) > > that builds clean. keep in mind I just compile tested, no rebooting or > anything. Should I just resend with what you posted, and call it that, > or is this something that needs more? I think it would be better to first decide what to do about the new warnings. Thanks.
On 06/30/2010 12:11 AM, Tejun Heo wrote: > On 06/30/2010 09:06 AM, Justin P. Mattock wrote: >>> Hmm, is this something we wanna go around the kernel and updating >>> sources? Or should we just flick a gcc option? >> >> the flick a gcc option seems easier todo, but my guess its probably not >> the right way of dealing with the issue(even a warning). > > What does the extra warning buy us? There are several places which > use anonymous enums for constants and I can't see what the benefit of > this warning would be. > I don't think it buys us anything..think it's just saying "hey you have two #defines with the same value" or something in that area(if Im reading the warning correctly) funny thing is, is gcc should of done the same with the original patch that I sent, as well as the second.. (but could be wrong). >>> And, just do WARN_ON((int)ATA_MAX_QUEUE> (int)AHCI_MAX_CMDS) >> >> that builds clean. keep in mind I just compile tested, no rebooting or >> anything. Should I just resend with what you posted, and call it that, >> or is this something that needs more? > > I think it would be better to first decide what to do about the > new warnings. > > Thanks. > sure.. no problem. cheers, Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 06/30/2010 09:21 AM, Justin P. Mattock wrote: >> What does the extra warning buy us? There are several places which >> use anonymous enums for constants and I can't see what the benefit of >> this warning would be. > > I don't think it buys us anything..think it's just saying "hey > you have two #defines with the same value" or something in that > area(if Im reading the warning correctly) funny thing is, is > gcc should of done the same with the original patch that I > sent, as well as the second.. (but could be wrong). Okay, just tested and it's warning that the code is comparing two enums which are from two different enum blocks. I'm kind of doubtful about its usefulness but then again I don't think there will be many occassions of this to be problematic either. For now, can you please submit a patch to cast just ATA_MAX_QUEUE to (int) before comparing and short comment explanining why it's necessary? Thanks.
On 07/03/2010 02:57 AM, Tejun Heo wrote: > Hello, > > On 06/30/2010 09:21 AM, Justin P. Mattock wrote: >>> What does the extra warning buy us? There are several places which >>> use anonymous enums for constants and I can't see what the benefit of >>> this warning would be. >> >> I don't think it buys us anything..think it's just saying "hey >> you have two #defines with the same value" or something in that >> area(if Im reading the warning correctly) funny thing is, is >> gcc should of done the same with the original patch that I >> sent, as well as the second.. (but could be wrong). > > Okay, just tested and it's warning that the code is comparing two > enums which are from two different enum blocks. I'm kind of > doubtful about its usefulness but then again I don't think there > will be many occassions of this to be problematic either. For > now, can you please submit a patch to cast just ATA_MAX_QUEUE > to (int) before comparing and short comment explanining why it's > necessary? > > Thanks. > o.k., just sent a patch subject is: Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>' hopefully it's doing what you had asked it to do i.g. send ATA_MAX_QUEUE to int. let me know if it needs to be resent or anything. Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/03/2010 02:57 AM, Tejun Heo wrote: > Hello, > > On 06/30/2010 09:21 AM, Justin P. Mattock wrote: >>> What does the extra warning buy us? There are several places which >>> use anonymous enums for constants and I can't see what the benefit of >>> this warning would be. >> >> I don't think it buys us anything..think it's just saying "hey >> you have two #defines with the same value" or something in that >> area(if Im reading the warning correctly) funny thing is, is >> gcc should of done the same with the original patch that I >> sent, as well as the second.. (but could be wrong). > > Okay, just tested and it's warning that the code is comparing two > enums which are from two different enum blocks. I'm kind of > doubtful about its usefulness but then again I don't think there > will be many occassions of this to be problematic either. For > now, can you please submit a patch to cast just ATA_MAX_QUEUE > to (int) before comparing and short comment explanining why it's > necessary? > > Thanks. > o.k. so it's what you where asking.. cheers, Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 8ca16f5..68c9add 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1038,11 +1038,11 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) struct device *dev = &pdev->dev; struct ahci_host_priv *hpriv; struct ata_host *host; - int n_ports, i, rc; + int n_ports, i, rc, sum = AHCI_MAX_CMDS; VPRINTK("ENTER\n"); - WARN_ON(ATA_MAX_QUEUE > AHCI_MAX_CMDS); + WARN_ON(ATA_MAX_QUEUE > sum); if (!printed_version++) dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
The below patch fixes a warning message during compiling of the kernel CC drivers/ata/ahci.o drivers/ata/ahci.c: In function 'ahci_init_one': drivers/ata/ahci.c:1045:2: warning: comparison between 'enum <anonymous>' and 'enum <anonymous>' Signed-off-by: Justin P. Mattock <justinmattock@gmail.com> --- drivers/ata/ahci.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)