2007-10-30 10:54:38

by Richard Knutsson

[permalink] [raw]
Subject: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

Convert to use the generic boolean.

Signed-off-by: Richard Knutsson <[email protected]>
---
Diffed against linus-git
Checked with script/checkpatch.pl
(warned about long lines, but it is not introduced by this patch)

dpt_i2o.c | 22 +++++++++++-----------
dpti.h | 9 ++-------
2 files changed, 13 insertions(+), 18 deletions(-)


diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 8258506..bd3a82d 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -252,7 +252,7 @@ rebuild_sys_tab:
adpt_i2o_delete_hba(pHba);
continue;
}
- pHba->initialized = TRUE;
+ pHba->initialized = true;
pHba->state &= ~DPTI_STATE_RESET;
scsi_scan_host(pHba->host);
}
@@ -519,7 +519,7 @@ static int adpt_proc_info(struct Scsi_Host *host, char *buffer, char **start, of
int unit;

*start = buffer;
- if (inout == TRUE) {
+ if (inout) {
/*
* The user has done a write and wants us to take the
* data in the buffer and do something with it.
@@ -893,7 +893,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
void __iomem *base_addr_virt = NULL;
void __iomem *msg_addr_virt = NULL;

- int raptorFlag = FALSE;
+ bool raptorFlag = false;

if(pci_enable_device(pDev)) {
return -EINVAL;
@@ -926,7 +926,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
// Use BAR1 in this configuration
base_addr1_phys = pci_resource_start(pDev,1);
hba_map1_area_size = pci_resource_len(pDev,1);
- raptorFlag = TRUE;
+ raptorFlag = true;
}

base_addr_virt = ioremap(base_addr0_phys,hba_map0_area_size);
@@ -936,7 +936,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
return -EINVAL;
}

- if(raptorFlag == TRUE) {
+ if (raptorFlag) {
msg_addr_virt = ioremap(base_addr1_phys, hba_map1_area_size );
if (!msg_addr_virt) {
PERROR("dpti: adpt_config_hba: io remap failed on BAR1\n");
@@ -996,7 +996,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
spin_lock_init(&pHba->state_lock);
spin_lock_init(&adpt_post_wait_lock);

- if(raptorFlag == 0){
+ if (!raptorFlag) {
printk(KERN_INFO"Adaptec I2O RAID controller %d at %p size=%x irq=%d\n",
hba_count-1, base_addr_virt, hba_map0_area_size, pDev->irq);
} else {
@@ -1273,7 +1273,7 @@ static s32 adpt_i2o_reset_hba(adpt_hba* pHba)
u32 m = EMPTY_QUEUE ;
ulong timeout = jiffies + (TMOUT_IOPRESET*HZ);

- if(pHba->initialized == FALSE) { // First time reset should be quick
+ if (!pHba->initialized) { /* First time reset should be quick */
timeout = jiffies + (25*HZ);
} else {
adpt_i2o_quiesce_hba(pHba);
@@ -1576,7 +1576,7 @@ static int adpt_open(struct inode *inode, struct file *file)
// return -EBUSY;
// }

- pHba->in_use = 1;
+ pHba->in_use = true;
mutex_unlock(&adpt_configuration_lock);

return 0;
@@ -1602,7 +1602,7 @@ static int adpt_close(struct inode *inode, struct file *file)
return -ENXIO;
}

- pHba->in_use = 0;
+ pHba->in_use = false;

return 0;
}
@@ -2433,8 +2433,8 @@ static s32 adpt_i2o_reparse_lct(adpt_hba* pHba)
pDev->tid = tid;
memcpy(&d->lct_data, &lct->lct_entry[i], sizeof(i2o_lct_entry));
if (pDev->pScsi_dev) {
- pDev->pScsi_dev->changed = TRUE;
- pDev->pScsi_dev->removable = TRUE;
+ pDev->pScsi_dev->changed = true;
+ pDev->pScsi_dev->removable = true;
}
}
// Found it - mark it scanned
diff --git a/drivers/scsi/dpti.h b/drivers/scsi/dpti.h
index 0892f6c..5eb7274 100644
--- a/drivers/scsi/dpti.h
+++ b/drivers/scsi/dpti.h
@@ -155,11 +155,6 @@ static int adpt_device_reset(struct scsi_cmnd* cmd);
#define I2O_SCSI_DSC_QUEUE_FROZEN 0x4000


-#ifndef TRUE
-#define TRUE 1
-#define FALSE 0
-#endif
-
#define HBA_FLAGS_INSTALLED_B 0x00000001 // Adapter Was Installed
#define HBA_FLAGS_BLINKLED_B 0x00000002 // Adapter In Blink LED State
#define HBA_FLAGS_IN_RESET 0x00000040 /* in reset */
@@ -209,8 +204,8 @@ typedef struct _adpt_hba {
spinlock_t state_lock;
int unit;
int host_no; /* SCSI host number */
- u8 initialized;
- u8 in_use; /* is the management node open*/
+ bool initialized:8;
+ bool in_use:8; /* is the management node open*/

char name[32];
char detail[55];


2007-10-30 14:47:37

by Mark Salyzyn

[permalink] [raw]
Subject: RE: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

ACK

Sincerely -- Mark Salyzyn


> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of
> Richard Knutsson
> Sent: Tuesday, October 30, 2007 6:54 AM
> To: [email protected]
> Cc: [email protected]; [email protected];
> Richard Knutsson
> Subject: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
>
> Convert to use the generic boolean.
>
> Signed-off-by: Richard Knutsson <[email protected]>
> ---
> Diffed against linus-git
> Checked with script/checkpatch.pl
> (warned about long lines, but it is not introduced by this patch)
>
> dpt_i2o.c | 22 +++++++++++-----------
> dpti.h | 9 ++-------
> 2 files changed, 13 insertions(+), 18 deletions(-)
>
>
> diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
> index 8258506..bd3a82d 100644
> --- a/drivers/scsi/dpt_i2o.c
> +++ b/drivers/scsi/dpt_i2o.c
> @@ -252,7 +252,7 @@ rebuild_sys_tab:
> adpt_i2o_delete_hba(pHba);
> continue;
> }
> - pHba->initialized = TRUE;
> + pHba->initialized = true;
> pHba->state &= ~DPTI_STATE_RESET;
> scsi_scan_host(pHba->host);
> }
> @@ -519,7 +519,7 @@ static int adpt_proc_info(struct
> Scsi_Host *host, char *buffer, char **start, of
> int unit;
>
> *start = buffer;
> - if (inout == TRUE) {
> + if (inout) {
> /*
> * The user has done a write and wants us to take the
> * data in the buffer and do something with it.
> @@ -893,7 +893,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
> void __iomem *base_addr_virt = NULL;
> void __iomem *msg_addr_virt = NULL;
>
> - int raptorFlag = FALSE;
> + bool raptorFlag = false;
>
> if(pci_enable_device(pDev)) {
> return -EINVAL;
> @@ -926,7 +926,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
> // Use BAR1 in this configuration
> base_addr1_phys = pci_resource_start(pDev,1);
> hba_map1_area_size = pci_resource_len(pDev,1);
> - raptorFlag = TRUE;
> + raptorFlag = true;
> }
>
> base_addr_virt = ioremap(base_addr0_phys,hba_map0_area_size);
> @@ -936,7 +936,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
> return -EINVAL;
> }
>
> - if(raptorFlag == TRUE) {
> + if (raptorFlag) {
> msg_addr_virt = ioremap(base_addr1_phys,
> hba_map1_area_size );
> if (!msg_addr_virt) {
> PERROR("dpti: adpt_config_hba: io remap
> failed on BAR1\n");
> @@ -996,7 +996,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
> spin_lock_init(&pHba->state_lock);
> spin_lock_init(&adpt_post_wait_lock);
>
> - if(raptorFlag == 0){
> + if (!raptorFlag) {
> printk(KERN_INFO"Adaptec I2O RAID controller %d
> at %p size=%x irq=%d\n",
> hba_count-1, base_addr_virt,
> hba_map0_area_size, pDev->irq);
> } else {
> @@ -1273,7 +1273,7 @@ static s32 adpt_i2o_reset_hba(adpt_hba* pHba)
> u32 m = EMPTY_QUEUE ;
> ulong timeout = jiffies + (TMOUT_IOPRESET*HZ);
>
> - if(pHba->initialized == FALSE) { // First time
> reset should be quick
> + if (!pHba->initialized) { /* First time reset
> should be quick */
> timeout = jiffies + (25*HZ);
> } else {
> adpt_i2o_quiesce_hba(pHba);
> @@ -1576,7 +1576,7 @@ static int adpt_open(struct inode
> *inode, struct file *file)
> // return -EBUSY;
> // }
>
> - pHba->in_use = 1;
> + pHba->in_use = true;
> mutex_unlock(&adpt_configuration_lock);
>
> return 0;
> @@ -1602,7 +1602,7 @@ static int adpt_close(struct inode
> *inode, struct file *file)
> return -ENXIO;
> }
>
> - pHba->in_use = 0;
> + pHba->in_use = false;
>
> return 0;
> }
> @@ -2433,8 +2433,8 @@ static s32 adpt_i2o_reparse_lct(adpt_hba* pHba)
> pDev->tid = tid;
>
> memcpy(&d->lct_data, &lct->lct_entry[i], sizeof(i2o_lct_entry));
> if (pDev->pScsi_dev) {
> -
> pDev->pScsi_dev->changed = TRUE;
> -
> pDev->pScsi_dev->removable = TRUE;
> +
> pDev->pScsi_dev->changed = true;
> +
> pDev->pScsi_dev->removable = true;
> }
> }
> // Found it - mark it scanned
> diff --git a/drivers/scsi/dpti.h b/drivers/scsi/dpti.h
> index 0892f6c..5eb7274 100644
> --- a/drivers/scsi/dpti.h
> +++ b/drivers/scsi/dpti.h
> @@ -155,11 +155,6 @@ static int adpt_device_reset(struct
> scsi_cmnd* cmd);
> #define I2O_SCSI_DSC_QUEUE_FROZEN 0x4000
>
>
> -#ifndef TRUE
> -#define TRUE 1
> -#define FALSE 0
> -#endif
> -
> #define HBA_FLAGS_INSTALLED_B 0x00000001 //
> Adapter Was Installed
> #define HBA_FLAGS_BLINKLED_B 0x00000002 //
> Adapter In Blink LED State
> #define HBA_FLAGS_IN_RESET 0x00000040 /* in reset */
> @@ -209,8 +204,8 @@ typedef struct _adpt_hba {
> spinlock_t state_lock;
> int unit;
> int host_no; /* SCSI host number */
> - u8 initialized;
> - u8 in_use; /* is the management node open*/
> + bool initialized:8;
> + bool in_use:8; /* is the management node open*/
>
> char name[32];
> char detail[55];
> -
> To unsubscribe from this list: send the line "unsubscribe
> linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2007-10-30 14:52:14

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

On Tue, Oct 30, 2007 at 11:54:22AM +0100, Richard Knutsson wrote:
> Convert to use the generic boolean.
> - u8 initialized;
> - u8 in_use; /* is the management node open*/
> + bool initialized:8;
> + bool in_use:8; /* is the management node open*/

Are you serious?

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2007-10-30 15:09:04

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

Matthew Wilcox wrote:
> On Tue, Oct 30, 2007 at 11:54:22AM +0100, Richard Knutsson wrote:
>
>> Convert to use the generic boolean.
>> - u8 initialized;
>> - u8 in_use; /* is the management node open*/
>> + bool initialized:8;
>> + bool in_use:8; /* is the management node open*/
>>
>
> Are you serious?
>
Well, yes. It is since it was defined to really be 8 bits before, and
there is no reason why a boolean would be 8 bits and not 1 or 16.
If it is overly cautious/not needed, then I don't mind removing the ':8'...

Richard Knutsson


2007-10-30 15:11:31

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

On Tue, Oct 30, 2007 at 04:02:25PM +0100, Richard Knutsson wrote:
> Matthew Wilcox wrote:
> >On Tue, Oct 30, 2007 at 11:54:22AM +0100, Richard Knutsson wrote:
> >
> >>Convert to use the generic boolean.
> >>- u8 initialized;
> >>- u8 in_use; /* is the management node open*/
> >>+ bool initialized:8;
> >>+ bool in_use:8; /* is the management node open*/
> >>
> >
> >Are you serious?
> >
> Well, yes. It is since it was defined to really be 8 bits before, and
> there is no reason why a boolean would be 8 bits and not 1 or 16.
> If it is overly cautious/not needed, then I don't mind removing the ':8'...

What's wrong with leaving it as 'u8'?

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2007-10-30 16:52:43

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

Matthew Wilcox wrote:
> On Tue, Oct 30, 2007 at 04:02:25PM +0100, Richard Knutsson wrote:
>
>> Matthew Wilcox wrote:
>>
>>> On Tue, Oct 30, 2007 at 11:54:22AM +0100, Richard Knutsson wrote:
>>>
>>>
>>>> Convert to use the generic boolean.
>>>> - u8 initialized;
>>>> - u8 in_use; /* is the management node open*/
>>>> + bool initialized:8;
>>>> + bool in_use:8; /* is the management node open*/
>>>>
>>>>
>>> Are you serious?
>>>
>>>
>> Well, yes. It is since it was defined to really be 8 bits before, and
>> there is no reason why a boolean would be 8 bits and not 1 or 16.
>> If it is overly cautious/not needed, then I don't mind removing the ':8'...
>>
>
> What's wrong with leaving it as 'u8'?
>
I just don't see the reason why expressing a boolean as an integer. Some
advantage?
(also helps us if someone does: 'if (var == true)', even thou we should
try to avoid them)

2007-10-30 17:38:39

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

On Tue, Oct 30, 2007 at 05:46:08PM +0100, Richard Knutsson wrote:
> I just don't see the reason why expressing a boolean as an integer. Some
> advantage?

This is C, not Java, or some other highly-typed language.
if (int) and if (ptr) are perfectly acceptable in C.

> (also helps us if someone does: 'if (var == true)', even thou we should
> try to avoid them)

I have no idea what you mean.

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2007-10-30 20:59:20

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean

Matthew Wilcox wrote:
> On Tue, Oct 30, 2007 at 05:46:08PM +0100, Richard Knutsson wrote:
>
>> I just don't see the reason why expressing a boolean as an integer. Some
>> advantage?
>>
>
> This is C, not Java, or some other highly-typed language.
> if (int) and if (ptr) are perfectly acceptable in C.
>
Yes, and it has been "corrected" in the C99-standard (which we are using
since the Linux support for the the 2.95 compiler stopped). Is there
something wrong in actually typing the variable to the type we want it
to be? Or would it be better to regress (becoming like Perl or PHP)[1][2]?
Btw, I am all for 'if (ptr)' since it rarely (if ever) makes any sense
to do 'if (ptr == 0x1234)', which evaluates to ptr == NULL is invalid
and otherwise valid. Is it really the same for integers? ;)
(also just want to add: to the "this is not a highly-typed language"
that I have heard many times, it theory, I think it would suffice with
just the void-pointer)
>
>> (also helps us if someone does: 'if (var == true)', even thou we should
>> try to avoid them)
>>
>
> I have no idea what you mean.
>
There is some places where things like 'if (var == true)' is done, but
what happens when var is not the same number as 'true' (and still != 0)?
It is a potential bug if 'var' is an integer and expected to be a
boolean in this case. Like in a case of "var = some_var & some_flag;"

Best regards
Richard Knutsson
[1] Nothing wrong with Perl or PHP, they are suited for the tasks they
are meant to solve.
[2] If I recall correctly, the predecessor 'B' did not have any types.