linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: "Uwe Kleine-König" <uwe@kleine-koenig.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
Date: Mon, 26 Aug 2019 14:05:44 +0200	[thread overview]
Message-ID: <20190826120544.ccbtjkgvk3ao4ak6@pathway.suse.cz> (raw)
In-Reply-To: <20190825091442.GA5817@taurus.defre.kleine-koenig.org>

On Sun 2019-08-25 11:14:42, Uwe Kleine-König  wrote:
> Hello Andrew,
> 
> On Sat, Aug 24, 2019 at 04:58:29PM -0700, Andrew Morton wrote:
> > (cc printk maintainers).
> 
> Ah, I wasn't aware there is something like them. Thanks
> 
> > On Sun, 25 Aug 2019 01:37:23 +0200 Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> > 
> > > 	pr_info("probing failed (%dE)\n", ret);
> > > 
> > > expands to
> > > 
> > > 	probing failed (EIO)
> > > 
> > > if ret holds -EIO (or EIO). This introduces an array of error codes. If
> > > the error code is missing, %dE falls back to %d and so prints the plain
> > > number.

What was the motivation for this patch, please?

Did it look like a good idea?
Did anyone got tired by searching for the error codes many
times a day?
Did the idea came from a developer, support, or user, please?

> 	add/remove: 2/0 grow/shrink: 4/2 up/down: 1488/-8 (1480)
> 	Function                                     old     new   delta
> 	errorcodes                                     -    1200   +1200
> 	errstr                                         -     200    +200
> 	vsnprintf                                    884     960     +76
> 	set_precision                                148     152      +4
> 	resource_string                             1380    1384      +4
> 	flags_string                                 400     404      +4
> 	num_to_str                                   288     284      -4
> 	format_decode                               1024    1020      -4
> 	Total: Before=21686, After=23166, chg +6.82%
> 
> But that doesn't seem to include the size increase for all the added
> strings which seems to be around another 1300 bytes.

This non-trivial increase of the size and the table still
includes only part of the error codes.

The array is long, created by cpu&paste, the index of each code
is not obvious.

There are ideas to make the code even more tricky to reduce
the size, keep it fast.

Both, %dE modifier and the output format (ECODE) is non-standard.

Upper letters gain a lot of attention. But the error code is
only helper information. Also many error codes are misleading because
they are used either wrongly or there was no better available.

There is no proof that this approach would be widely acceptable for
subsystem maintainers. Some might not like mass and "blind" code
changes. Some might not like the output at all.

I am not persuaded that all this is worth it. Also I do not like
the non-standard solution.

Best Regards,
Petr

  reply	other threads:[~2019-08-26 12:05 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-24 23:37 [PATCH v1 1/2] vsprintf: introduce %dE for error constants Uwe Kleine-König
2019-08-24 23:37 ` [PATCH v1 2/2] gpiolib: print an error name instead of a plain number in error string Uwe Kleine-König
2019-08-24 23:58 ` [PATCH v1 1/2] vsprintf: introduce %dE for error constants Andrew Morton
2019-08-25  9:14   ` Uwe Kleine-König
2019-08-26 12:05     ` Petr Mladek [this message]
2019-08-26  5:55   ` Sergey Senozhatsky
2019-08-26  9:58   ` Jani Nikula
2019-08-26 10:04     ` Jani Nikula
2019-08-26 10:13 ` Rasmus Villemoes
2019-08-26 13:29 ` Enrico Weigelt, metux IT consult
2019-08-30 13:21   ` David Laight
2019-08-29 13:27 ` Andy Shevchenko

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=20190826120544.ccbtjkgvk3ao4ak6@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=bgolaszewski@baylibre.com \
    --cc=corbet@lwn.net \
    --cc=linus.walleij@linaro.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=uwe@kleine-koenig.org \
    /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;
as well as URLs for NNTP newsgroup(s).