Re: [PATCH v2 1/2] lib/vsprintf.c: Simplify uuid_string()

George Spelvin
2016-06-05T19:57:46Z

r
>From andriy.shevchenko@linux.intel.com Sun Jun 05 14:21:40 2016
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.26,421,1459839600";
d="scan'208";a="969274163"
Subject: Re: [PATCH v2 1/2] lib/vsprintf.c: Simplify uuid_string()
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: George Spelvin <linux@sciencehorizons.net>
Cc: bjorn@mork.no, linux-kernel@vger.kernel.org, matt@codeblueprint.co.uk,
rv@rasmusvillemoes.dk
Date: Sun, 05 Jun 2016 17:22:56 +0300
In-Reply-To: <20160604051411.3635.qmail@ns.sciencehorizons.net>
References: <20160604051411.3635.qmail@ns.sciencehorizons.net>
Organization: Intel Finland Oy
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.20.2-2
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit

Andy Shevchenko wrote:
> On Sat, 2016-06-04 at 01:14 -0400, George Spelvin wrote:
>> - if (uc)
>> - p = hex_byte_pack_upper(p, addr[index[i]]);
>> - else
>> - p = hex_byte_pack(p, addr[index[i]]);
>> + u8 byte = addr[index[i]];
>> +
>> + *p++ = hex[byte >> 4];
>> + *p++ = hex[byte & 0x0f];

> And what prevents you to assign hex_byte_pack()/hex_byte_pack_upper()
> and do one call here?

Because they're inline functions, so there's no compiled copy to
take the address of.

Since they're delcared "static", if you take their addresses anyway,
gcc will helpfully compile private versions for the use of this source
file, which will be bigger and slower than the simple inline expansion
I used here.

It's not even any *simpler*. Remembering what "hex_byte_pack()" means
is as much mental effort as interpreting those two very simple lines.

There's no strong reason to *avoid* using the hex_asc[] arrays directly.
It's done in several other places in the kernel, including earlier in
lib/vsprintf.c (search for "hex_asc_upper" in number()).

If they were intended to be "off limits", they would have been given
_-prefixed names.