[PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Justin P. Mattock]

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(-)

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");
-- 
1.7.1.rc1.21.gf3bd6

Re: [PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Tejun Heo]

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.

-- 
tejun

Re: [PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Justin P. Mattock]

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

Re: [PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Tejun Heo]

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.

-- 
tejun

Re: [PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Justin P. Mattock]

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

Re: [PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Tejun Heo]

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.

-- 
tejun

Re: [PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Justin P. Mattock]

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

Re: [PATCH v2]ata:ahci.c Fix warning: comparison between 'enum <anonymous>' and 'enum <anonymous>'

[Justin P. Mattock]

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