lkml.org 
[lkml]   [2009]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 6/7] vsprintf: move local vars to block local vars and remove unneeded ones
On Mon, Nov 02, 2009 at 03:26:48PM -0200, André Goddard Rosa wrote:
> From 636fc10ec894c83ba3b73433fee6df94529bc930 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= <andre.goddard@gmail.com>
> Date: Sun, 1 Nov 2009 14:04:37 -0200
> Subject: [PATCH v2 6/7] vsprintf: move local vars to block local vars
> and remove unneeded ones
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> It also decreases code size:
> text data bss dec hex filename
> 15719 0 8 15727 3d6f lib/vsprintf.o-before
> 15703 0 8 15711 3d5f lib/vsprintf.o-after



Actually, moving variable scope from function local to block
local is nice for code reviewing, it increases the code readability.
And this is a more important impact than binary code size IMO.



> @@ -1616,7 +1609,9 @@ int bstr_printf(char *buf, size_t size, const
> char *fmt, const u32 *bin_buf)
> spec.precision = get_arg(int);
> break;
>
> - case FORMAT_TYPE_CHAR:
> + case FORMAT_TYPE_CHAR: {
> + char c;
> +
> if (!(spec.flags & LEFT)) {
> while (--spec.field_width > 0) {
> if (str < end)
> @@ -1634,11 +1629,11 @@ int bstr_printf(char *buf, size_t size, const
> char *fmt, const u32 *bin_buf)
> ++str;
> }
> break;
> + }
>
> case FORMAT_TYPE_STR: {
> const char *str_arg = args;
> - size_t len = strlen(str_arg);
> - args += len + 1;
> + args += strlen(str_arg) + 1;
> str = string(str, end, (char *)str_arg, spec);
> break;
> }
> @@ -1655,6 +1650,10 @@ int bstr_printf(char *buf, size_t size, const
> char *fmt, const u32 *bin_buf)
> ++str;
> break;
>
> + /*
> + * Merging this handling with the above one increases code size!
> + * Why, gcc, why?!
> + */
> case FORMAT_TYPE_INVALID:



Please do this merge:

@@ -1633,11 +1633,6 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
break;

case FORMAT_TYPE_PERCENT_CHAR:
- if (str < end)
- *str = '%';
- ++str;
- break;
-
case FORMAT_TYPE_INVALID:
if (str < end)
*str = '%';

We don't care that much about binary code size in vsprintf.c
Code readability has a really higher priority over binary code size
here.


The rest looks good.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-11-02 19:55    [W:0.031 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site