Patchwork [06/10] mtd-utils: add common version printing function

login
register
mail settings
Submitter Brian Norris
Date June 27, 2011, 6:27 p.m.
Message ID <1309199247-19248-7-git-send-email-computersforpeace@gmail.com>
Download mbox | patch
Permalink /patch/102249/
State New
Headers show

Comments

Brian Norris - June 27, 2011, 6:27 p.m.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 include/common.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Mike Frysinger - June 27, 2011, 7:51 p.m.
On Mon, Jun 27, 2011 at 14:27, Brian Norris wrote:
> +/* Simple version-printing for utils */
> +#define common_print_version() \
> +do { \
> +       fprintf(stderr, PROGRAM_NAME " " VERSION "\n"); \
> +} while (0)

this shouldnt go to stderr

if the string is const, then it could just be puts().  but i wonder if
it shouldnt instead use printf("%s %s\n", PROGRAM_NAME, VERSION) so
that the string that represents PROGRAM_NAME isn't duplicated in the
.rodata section.
-mike
Brian Norris - June 27, 2011, 8:15 p.m.
On Mon, Jun 27, 2011 at 12:51 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Mon, Jun 27, 2011 at 14:27, Brian Norris wrote:
>> +/* Simple version-printing for utils */
>> +#define common_print_version() \
>> +do { \
>> +       fprintf(stderr, PROGRAM_NAME " " VERSION "\n"); \
>> +} while (0)
>
> this shouldnt go to stderr

Yeah, I wasn't too sure on that one. There were several utils that
already used stderr, but that doesn't actually make much sense, I
guess.

> if the string is const, then it could just be puts().  but i wonder if
> it shouldnt instead use printf("%s %s\n", PROGRAM_NAME, VERSION) so
> that the string that represents PROGRAM_NAME isn't duplicated in the
> .rodata section.

I'm not sure I know what the exact technical difference would be
between the string concatenation version and the '%s' argument
version...

Brian
Mike Frysinger - June 27, 2011, 8:20 p.m.
On Mon, Jun 27, 2011 at 16:15, Brian Norris wrote:
> On Mon, Jun 27, 2011 at 12:51 PM, Mike Frysinger wrote:
>> if the string is const, then it could just be puts().  but i wonder if
>> it shouldnt instead use printf("%s %s\n", PROGRAM_NAME, VERSION) so
>> that the string that represents PROGRAM_NAME isn't duplicated in the
>> .rodata section.
>
> I'm not sure I know what the exact technical difference would be
> between the string concatenation version and the '%s' argument
> version...

"%s %s\n" is one string in .rodata, and PROGRAM_NAME is another.  so
anytime the app uses PROGRAM_NAME (like in all the helpers in
common.h), they'll reuse that same PROGRAM_NAME string rather than
constantly duplicating it.

if however you use PROGRAM_NAME " " VERSION, you have a unique string
that is only used by the version code.

minor details, but all these duplicate const strings slowly add up ...
-mike

Patch

diff --git a/include/common.h b/include/common.h
index 65ec086..b285bc0 100644
--- a/include/common.h
+++ b/include/common.h
@@ -129,6 +129,12 @@  simple_strtoX(strtoll, long long int)
 simple_strtoX(strtoul, unsigned long int)
 simple_strtoX(strtoull, unsigned long long int)
 
+/* Simple version-printing for utils */
+#define common_print_version() \
+do { \
+	fprintf(stderr, PROGRAM_NAME " " VERSION "\n"); \
+} while (0)
+
 #include "xalloc.h"
 
 #ifdef __cplusplus