Skip to contentSkip to conversation

int to bool conversion

PATCH In review Quentin Lambert 3 participants 5 messages Jan 22, 2015 – Feb 18, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover int to bool conversion

These patches convert local variables from int to bool when relevant.

The first patch proposes straight forward cases where the conversion do not suggest changes beyond the function.

The second patch deals with cases where the conversion is propagated to function parameters.

The third patch is a single case where the variable is given as a parameter to a kernel macro.

Finally, the fourth patch is a case in which the conversion is propagated to the field of a structure.

Quentin Lambert (4):
ACPI: Convert non-returned local variable to boolean when relevant
ACPI: Convert int variable to bool and propagate to function
parameters
ACPI: Convert int to bool for variable later used int kernel macro
ACPI: Convert int to bool and propagete to struct field

 drivers/acpi/acpi_pad.c         |  6 +++---
 drivers/acpi/acpi_processor.c   |  5 +++--
 drivers/acpi/acpica/acutils.h   |  3 ++-
 drivers/acpi/acpica/utaddress.c |  3 ++-
 drivers/acpi/acpica/utstring.c  | 10 +++++-----
 drivers/acpi/acpica/utxface.c   |  2 +-
 drivers/acpi/dock.c             |  4 ++--
 drivers/acpi/osl.c              |  4 ++--
 drivers/acpi/pci_link.c         |  4 ++--
 drivers/acpi/processor_core.c   | 12 ++++++------
 drivers/acpi/processor_pdc.c    |  5 +++--
 drivers/acpi/scan.c             |  4 ++--
 drivers/acpi/thermal.c          | 10 +++++-----
 13 files changed, 38 insertions(+), 34 deletions(-)
Signature · 1 line
1.9.1
#1 lore raw
Quoted 1 line · Quentin Lambert

These patches convert local variables from int to bool when relevant.

And what exactly is the need for that? Does that fix any functional problems?

Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#2 lore raw

Sorry for the delay in answering ....

Quoted 5 lines · Rafael J. Wysocki
Quoted 1 line · Quentin Lambert

These patches convert local variables from int to bool when relevant.

And what exactly is the need for that? Does that fix any functional problems?

It doesn't fix any functional problem. The point of this patch is to increase the code readability by lifting some of the ambiguities that appear when using an integer variable as boolean.

My understanding is that by explicitly using a boolean declaration when it is relevant it clearly informs the reader that the variable is going to represent a binary state. Moreover, using the keywords true and false help indicate that the variable will not be involved in any computation other than boolean arithmetic.

That being said, I am new to kernel contribution and I feel that a more compelling case may be made here:
http://lkml.iu.edu/hypermail/linux/kernel/0607.2/0791.html

BR,
Quentin

#3 lore raw
Quoted 12 lines · Quentin Lambert

Sorry for the delay in answering ....

Quoted 5 lines · Rafael J. Wysocki
Quoted 1 line · Quentin Lambert

These patches convert local variables from int to bool when relevant.

And what exactly is the need for that? Does that fix any functional problems?

It doesn't fix any functional problem. The point of this patch is to increase the code readability by lifting some of the ambiguities that appear when using an integer variable as boolean.

Quoted 5 lines

My understanding is that by explicitly using a boolean declaration when it is relevant it clearly informs the reader that the variable is going to represent a binary state. Moreover, using the keywords true and false help indicate that the variable will not be involved in any computation other than boolean arithmetic.

Well, in the new code, yes. The existing code is a different matter though and it doesn't actually hurt if you leave the ints where they are, so there's no reason to make those changes.

If you change old code and the change is not trivial (eg. fixes of white space or comments, or kernel messages etc.) and someone enounters a bug that may be related to it, they will have to go through your changes to see if that's not the source of the bug. That's not really productive.

Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#4 lore raw

"bool" can be problematic as it isn't totally portable. It is usually implemented as a macro.

That’s why ACPICA doesn't use it.

Quoted 42 lines

-----Original Message-----
From: Rafael J. Wysocki [mailto:rjw(a)rjwysocki.net]
Sent: Monday, January 26, 2015 5:33 AM
To: Quentin Lambert
Cc: Zhang, Rui; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown; Shaohua Li; linux-acpi(a)vger.kernel.org; devel(a)acpica.org; linux- kernel(a)vger.kernel.org
Subject: Re: [PATCH 0/4] int to bool conversion

Quoted 6 lines · Quentin Lambert

Sorry for the delay in answering ....

Quoted 3 lines · Rafael J. Wysocki
Quoted 1 line · Quentin Lambert

These patches convert local variables from int to bool when relevant.

And what exactly is the need for that? Does that fix any functional

problems?

Quoted 11 lines
Quoted 2 lines

It doesn't fix any functional problem. The point of this patch is to increase the code readability by lifting some of the ambiguities that appear when using an integer variable as boolean.

My understanding is that by explicitly using a boolean declaration when it is relevant it clearly informs the reader that the variable is going to represent a binary state. Moreover, using the keywords true and false help indicate that the variable will not be involved in any computation other than boolean arithmetic.

Well, in the new code, yes. The existing code is a different matter though and it doesn't actually hurt if you leave the ints where they are, so there's no reason to make those changes.

If you change old code and the change is not trivial (eg. fixes of white space or comments, or kernel messages etc.) and someone enounters a bug that may be related to it, they will have to go through your changes to see if that's not the source of the bug. That's not really productive.

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.