utilities/: Compliment va_start() with va_end().

PATCH Moore, Robert 2 participants 4 messages Nov 26, 2007 – Nov 27, 2007 1 message not in the archive ACPI

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Moore, Robert

author

2007-11-26 22:14 UTC #1 lore raw

This is an interesting one to me.

Quoted 1 line

From various documentation:

After all arguments have been retrieved, va_end resets the pointer to NULL.

va_end
Each invocation of va_start must be matched by a corresponding invocation of va_end in the same function. After the call va_end(ap) the variable ap is undefined. Multiple transversals of the list, each bracketed by va_start and va_end are possible. va_end may be a macro or a function.

Now, I'm all for defensive programming, but I don't really see the point of va_end when the list will be only traversed once.

We don't set all local pointers to NULL at function exit, what is the point of doing it here?

I suppose some implementation could allocate memory at va_start, but in practice, does this happen?

Bob

Quoted 9 lines

-----Original Message-----
From: Len Brown [mailto:[email protected]]
Sent: Saturday, November 24, 2007 11:06 PM
To: Moore, Robert
Subject: Fwd: [PATCH] [ACPI] utilities/: Compliment va_start() with va_end().

Quoted 3 lines

---------- Forwarded Message ----------
Subject: [PATCH] [ACPI] utilities/: Compliment va_start() with

va_end().

drivers/acpi/utilities/utmisc.c acpi_ut_warning(char *module_name, u32

Quoted 61 lines

Date: Saturday 24 November 2007 15:43
From: Richard Knutsson <[email protected]>
To: [email protected]
Cc: [email protected], [email protected], Richard Knutsson <[email protected]>

Compliment va_start() with va_end().

Signed-off-by Richard Knutsson <[email protected]>

---
Compile-tested on i386 with allyesconfig & allmodconfig.

utdebug.c |    2 ++
utmisc.c  |    4 ++++
2 files changed, 6 insertions(+)

drivers/acpi/utilities/utdebug.c +2 -0

index c7e128e..f45e3d5 100644
@@ -203,6 +203,7 @@ acpi_ut_debug_print(u32 requested_debug_level,
 
 	va_start(args, format);
 	acpi_os_vprintf(format, args);
+	va_end(args);
 }
 
 ACPI_EXPORT_SYMBOL(acpi_ut_debug_print)

@@ -240,6 +241,7 @@ acpi_ut_debug_print_raw(u32 requested_debug_level,
 
 	va_start(args, format);
 	acpi_os_vprintf(format, args);
+	va_end(args);
 }
 
 ACPI_EXPORT_SYMBOL(acpi_ut_debug_print_raw)

drivers/acpi/utilities/utmisc.c +2 -0

index 2d19f71..ca4904c 100644
@@ -1032,6 +1032,7 @@ acpi_ut_error(char *module_name, u32 line_number,
 char *format, ...)
 
 	va_start(args, format);
 	acpi_os_vprintf(format, args);
+	va_end(args);
 	acpi_os_printf(" [%X]\n", ACPI_CA_VERSION);
 }

@@ -1046,6 +1047,7 @@ acpi_ut_exception(char *module_name,
 
 	va_start(args, format);
 	acpi_os_vprintf(format, args);
+	va_end(args);
 	acpi_os_printf(" [%X]\n", ACPI_CA_VERSION);
 }

@@ -1060,6 +1062,7 @@ acpi_ut_warning(char *module_name, u32

line_number,

drivers/acpi/utilities/utmisc.c acpi_ut_info(char *module_name, u32 line_number,

Quoted 9 lines

drivers/acpi/utilities/utmisc.c +1 -0

 char *format, ...)
 
 	va_start(args, format);
 	acpi_os_vprintf(format, args);
+	va_end(args);
 	acpi_os_printf(" [%X]\n", ACPI_CA_VERSION);
 }

@@ -1076,5 +1079,6 @@ acpi_ut_info(char *module_name, u32 line_number,

char

Quoted 9 lines

drivers/acpi/utilities/utmisc.c +1 -0
 *format, ...)
 
 	va_start(args, format);
 	acpi_os_vprintf(format, args);
+	va_end(args);
 	acpi_os_printf("\n");
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel"

Quoted 6 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-------------------------------------------------------

Richard Knutsson

2007-11-27 00:28 UTC #2 lore raw

Quoted 14 lines · Robert

This is an interesting one to me.
From various documentation:
After all arguments have been retrieved, va_end resets the pointer to NULL.
va_end
Each invocation of va_start must be matched by a corresponding invocation of va_end in the same function. After the call va_end(ap) the variable ap is undefined. Multiple transversals of the list, each bracketed by va_start and va_end are possible. va_end may be a macro or a function.

Quoted 4 lines

Now, I'm all for defensive programming, but I don't really see the point of va_end when the list will be only traversed once.

First off, I think it is a good idea to follow the documentation, which stated:
"va_end
Each invocation of va_start must be matched by a corresponding invocation of va_end in the same function."

Then if it is not really needed, does it take up extra cycles? "In practice, with most C compilers, calling |va_end| does nothing and you do not really need to call it. This is always true in the GNU C compiler."[1]

Portability:
"But you might as well call |va_end| just in case your
program is someday compiled with a peculiar compiler."[2]
This argument is not as likely thou, but who knows? (Since I guess Intel's compiler is included in the 'most C compilers')

Quoted 4 lines

We don't set all local pointers to NULL at function exit, what is the point of doing it here?

I think it is a good thing if the code follows the documentation, both for the person who tries to understand the code (to see when the 'args' is no longer needed and not getting confused by the absent of va_end(), after all, IMHO we should write the code how we want things to work and let the compiler do the optimizations (it usually does a better job at it then we do)) and to automated searches (that is how I found this one).

Quoted 3 lines

I suppose some implementation could allocate memory at va_start, but in practice, does this happen?

Not sure what you mean.

Quoted 3 lines

Bob

cu
Richard Knutsson

[1] http://www.cs.utah.edu/dept/old/texinfo/glibc-manual-0.02/library_28.html
[2] The rest of [1]'s line.

Moore, Robert

author

2007-11-27 00:52 UTC #3 lore raw

Yes, it's official ANSI C, so I agree with the portability. I'm probably asking more about the history of the thing.

Quoted 8 lines

-----Original Message-----
From: Richard Knutsson [mailto:[email protected]]
Sent: Monday, November 26, 2007 4:16 PM
To: Moore, Robert
Cc: Len Brown; [email protected]; [email protected]
Subject: Re: [PATCH] [ACPI] utilities/: Compliment va_start() with va_end().

Quoted 11 lines

Quoted 10 lines · Robert
This is an interesting one to me.
From various documentation:
After all arguments have been retrieved, va_end resets the pointer to NULL.
va_end
Each invocation of va_start must be matched by a corresponding invocation of va_end in the same function. After the call va_end(ap)

the

Quoted 2 lines

Quoted 2 lines
variable ap is undefined. Multiple transversals of the list, each bracketed by va_start and va_end are possible. va_end may be a macro

Quoted 3 lines

Quoted 3 lines
a function.
Now, I'm all for defensive programming, but I don't really see the

point

Quoted 9 lines

Quoted 3 lines
of va_end when the list will be only traversed once.
First off, I think it is a good idea to follow the documentation, which stated:
"va_end
Each invocation of va_start must be matched by a corresponding invocation of va_end in the same function."

Quoted 3 lines

Then if it is not really needed, does it take up extra cycles? "In practice, with most C compilers, calling |va_end| does nothing and you do not really need to call it. This is always true in the GNU

Quoted 6 lines

compiler."[1]
Portability:
"But you might as well call |va_end| just in case your program is someday compiled with a peculiar compiler."[2] This argument is not as likely thou, but who knows? (Since I guess

Intel's

Quoted 6 lines

compiler is included in the 'most C compilers')
Quoted 4 lines
We don't set all local pointers to NULL at function exit, what is the point of doing it here?

Quoted 7 lines

I think it is a good thing if the code follows the documentation, both for the person who tries
to understand the code (to see when the 'args' is no longer needed and not getting confused
by the absent of va_end(), after all, IMHO we should write the code how we want things to
work and let the compiler do the optimizations (it usually does a

better

Quoted 3 lines

job at it then we do))
and to automated searches (that is how I found this one).
Quoted 1 line
I suppose some implementation could allocate memory at va_start, but

Quoted 11 lines

Quoted 2 lines
practice, does this happen?
Not sure what you mean.
Quoted 3 lines
Bob
cu
Richard Knutsson
[1]
http://www.cs.utah.edu/dept/old/texinfo/glibc-manual-0.02/library_28.ht

Quoted 1 line

[2] The rest of [1]'s line.

Richard Knutsson

2007-11-27 02:23 UTC #4 lore raw

Quoted 4 lines · Robert

Yes, it's official ANSI C, so I agree with the portability. I'm probably asking more about the history of the thing.

"the history of the thing"? Sorry, you lost me there. I know there were a pre-ANSI version of va_start() & co., but they seemed quite messy. When it comes to va_end() and maintainers, they often seem positive to this. I guess the occasional lack off va_end() is usually an oversight.

Quoted 109 lines

Quoted 9 lines
-----Original Message-----
From: Richard Knutsson [mailto:[email protected]]
Sent: Monday, November 26, 2007 4:16 PM
To: Moore, Robert
Cc: Len Brown; [email protected]; [email protected]
Subject: Re: [PATCH] [ACPI] utilities/: Compliment va_start() with va_end().
Quoted 13 lines
Quoted 11 lines · Robert
This is an interesting one to me.
From various documentation:
After all arguments have been retrieved, va_end resets the pointer to NULL.
va_end
Each invocation of va_start must be matched by a corresponding invocation of va_end in the same function. After the call va_end(ap)
the
Quoted 3 lines
Quoted 3 lines
variable ap is undefined. Multiple transversals of the list, each bracketed by va_start and va_end are possible. va_end may be a macro
or
Quoted 4 lines
Quoted 4 lines
a function.
Now, I'm all for defensive programming, but I don't really see the
point
Quoted 10 lines
Quoted 4 lines
of va_end when the list will be only traversed once.
First off, I think it is a good idea to follow the documentation, which stated:
"va_end
Each invocation of va_start must be matched by a corresponding invocation of va_end in the same function."
Quoted 4 lines
Then if it is not really needed, does it take up extra cycles? "In practice, with most C compilers, calling |va_end| does nothing and you do not really need to call it. This is always true in the GNU
C
Quoted 7 lines
compiler."[1]
Portability:
"But you might as well call |va_end| just in case your program is someday compiled with a peculiar compiler."[2] This argument is not as likely thou, but who knows? (Since I guess
Intel's
Quoted 7 lines
compiler is included in the 'most C compilers')
Quoted 4 lines
We don't set all local pointers to NULL at function exit, what is the point of doing it here?
Quoted 8 lines
I think it is a good thing if the code follows the documentation, both for the person who tries
to understand the code (to see when the 'args' is no longer needed and not getting confused
by the absent of va_end(), after all, IMHO we should write the code how we want things to
work and let the compiler do the optimizations (it usually does a
better
Quoted 5 lines
job at it then we do))
and to automated searches (that is how I found this one).
Quoted 2 lines
I suppose some implementation could allocate memory at va_start, but
in
Quoted 9 lines
Quoted 3 lines
practice, does this happen?
Not sure what you mean.
Quoted 4 lines
Bob
Quoted 6 lines
cu
Richard Knutsson
[1]
http://www.cs.utah.edu/dept/old/texinfo/glibc-manual-0.02/library_28.ht
ml
Quoted 2 lines
[2] The rest of [1]'s line.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/