public inbox for linux-kbuild@vger.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Daniel Santos <daniel.santos@pobox.com>
Cc: dhowells@redhat.com, linux-kbuild <linux-kbuild@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Michal Marek <mmarek@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	George Spelvin <linux@horizon.com>
Subject: Re: [PATCH 1/5] scripts: Add mkstrerror.sh
Date: Wed, 18 Sep 2013 12:38:20 +0100	[thread overview]
Message-ID: <29168.1379504300@warthog.procyon.org.uk> (raw)
In-Reply-To: <1379459317-13046-2-git-send-email-daniel.santos@pobox.com>

danielfsantos@att.net wrote:

> This is a simple bash script that parses our errno*.h files and formats
> them into the error_strings.h header that our strerror and strerror_name
> functions will use later.

I presume you haven't tried building with a "make O=foo" build directory?  I
see:

      /bin/sh: /data/fs/linux-2.6-fscache/include/generated/error_strings.h: No such file or directory

when I try it.

Looking in your generated error_strings.h file, I see:

	static const struct error_strings {
		const unsigned first;
		const unsigned last;
		const unsigned count;
		const char * const desc[2];
	} error_strings[2] = {
		{
			.first = 0,
			.last  = 133,
			.count = 134,
			.desc  = {
	#ifdef CONFIG_STRERROR_NAME
		"\0"                            /* 0 */
		"EPERM\0"                       /* 1 */
		"ENOENT\0"                      /* 2 */
		"ESRCH\0"                       /* 3 */
		"EINTR\0"                       /* 4 */
	...
		"ERFKILL\0"                     /* 132 */
		"EHWPOISON\0"                   /* 133 */,
	#else
		NULL,
	#endif /* CONFIG_STRERROR_NAME */

	#ifdef CONFIG_STRERROR
		"\0"                                                    /* 0 */
		"Operation not permitted\0"                             /* 1 */
		"No such file or directory\0"                           /* 2 */
		"No such process\0"                                     /* 3 */
	...
	        "State not recoverable\0"                               /* 131 */
		"Operation not possible due to RF-kill\0"               /* 132 */
		"Memory page has hardware error\0"                      /* 133 */,
	#else
		NULL,
	#endif /* CONFIG_STRERROR */
			},
		}, {
			.first = 512,
			.last  = 529,
			.count = 18,
			.desc  = {
	#ifdef CONFIG_STRERROR_NAME
		"ERESTARTSYS\0"                 /* 512 */
		"ERESTARTNOINTR\0"              /* 513 */
	...
		"Request initiated, but will not complete before timeout\0"/* 528 */
		"iocb queued, will get completion event\0"              /* 529 */,
	#else
		NULL,
	#endif /* CONFIG_STRERROR */
			},
		}
	};


Some thoughts for you:

 (1) Why are you double-NUL'ing all your strings?  (see the \0 in the strings)

 (2) Storing the leading 'E' for each symbol is redundant as you can add that
     later so you might want to discard it.

 (3) You are storing a pointer to the symbolic name for each error.  On a
     64-bit machine, that's 8 bytes.  If you drop the leading 'E' and the
     trailing NUL, most symbols will fit into an 8 character slot saving you
     the cost of a pointer.

     Okay, you'll have to truncate some of the names (ERESTARTNOINTR ->
     RESNOINT for example) but probably not that many.  Run this:

	grep '["]E[A-Z0-9]' include/generated/error_strings.h |
           cut '-d\' -f1 | cut -dE -f2- | cut -c1-8

     and you'll see what I mean.  Most of them are still recognisable,
     particularly once you stick the 'E' back on the front.  Piping the output
     of that through "wc -l", I get:

	147

     which means the entire string table could be squeezed into 8 * 147 or
     1176 bytes with only one pointer and one count required for each segment
     of the table (you generate two segments).

David

  reply	other threads:[~2013-09-18 11:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-17 23:08 [PATCH 0/5] Preliminary: Add error names & descrptions to printks danielfsantos
2013-09-17 23:08 ` [PATCH 1/5] scripts: Add mkstrerror.sh danielfsantos
2013-09-18 11:38   ` David Howells [this message]
2013-09-18 11:55     ` David Howells
2013-09-18 22:27       ` Daniel Santos
2013-09-18 22:43     ` Daniel Santos
2013-09-19 22:35     ` Daniel Santos
2013-09-19 22:53       ` Daniel Santos
2013-09-17 23:08 ` [PATCH 2/5] lib: Add .config options for error strings in printks danielfsantos
2013-09-17 23:18   ` Daniel Santos
2013-09-17 23:08 ` [PATCH 3/5] Makefile: Generate error_strings.h danielfsantos
2013-09-17 23:08 ` [PATCH 4/5] lib: Add strerror and strerror_name functions danielfsantos
2013-09-17 23:08 ` [PATCH 5/5] lib: Add error string support to printks danielfsantos
2013-09-18  5:36   ` Joe Perches
2013-09-18 11:04     ` David Howells
2013-09-19  1:27       ` Daniel Santos
2013-09-20  1:07         ` Joe Perches
2013-09-20  5:21           ` Daniel Santos
2013-09-20 11:45             ` Joe Perches
2013-09-20 13:07               ` David Howells
2013-09-23 20:17                 ` Daniel Santos
2013-09-23 19:40               ` Daniel Santos
2013-09-18 11:08 ` [PATCH 0/5] Preliminary: Add error names & descrptions " David Howells
2013-09-18 22:47   ` Daniel Santos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=29168.1379504300@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=daniel.santos@pobox.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@horizon.com \
    --cc=mmarek@suse.cz \
    --cc=mtk.manpages@gmail.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox