From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Mark Brown <broonie@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] regmap: debugfs: remove bogus check
Date: Wed, 30 Sep 2015 19:52:26 +0200 [thread overview]
Message-ID: <87d1wzg5gl.fsf@rasmusvillemoes.dk> (raw)
In-Reply-To: <20150930172045.GE15635@sirena.org.uk> (Mark Brown's message of "Wed, 30 Sep 2015 18:20:45 +0100")
On Wed, Sep 30 2015, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Sep 30, 2015 at 09:27:38AM +0200, Rasmus Villemoes wrote:
>
>> I agree, but only on the word 'great'. There is value in removing such
>> bogosities (or rather, their presence provides negative value). It makes
>> the code harder to read ("why is this instance checked, but not any of
>> the other snprintfs?"); people may think that it's trying to check for
>> truncation, but it does no such thing; and it contributes a few
>> worthless bytes to .text (and the source).
>
> The solution to partial error checking isn't always to remove the error
> checking!
Since this particular piece of code is not, in fact, doing any kind of error
checking, I fail to see how that argument is relevant.
Or, more constructively: Would you please explain exactly what error
checking you think this should/could be doing? Truncation? Invalid
format string? Cosmic-ray-detected?
>> If you're worried about map->dev->driver->name actually ever being > 2G,
>> returning some almost totally random negative number isn't really
>> helpful (the function is supposed to return a -errno). And what makes
>> you think that in some hypothetical universe where the kernel's snprintf
>> explicit returns a negative value that it wouldn't just return -1 (aka
>> -EPERM)?
>
> This is going back to the discussion about having to learn the specific
> snprinf() implementation one is dealing with.
You better know what values it might return under which circumstances if
you're going to forward those as error codes to your caller.
Rasmus
next prev parent reply other threads:[~2015-09-30 17:52 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-28 22:29 Rasmus Villemoes
2015-09-28 22:29 ` [PATCH 2/2] regmap: debugfs: improve regmap_reg_ranges_read_file() Rasmus Villemoes
2015-09-29 18:30 ` Mark Brown
2015-09-30 9:51 ` Rasmus Villemoes
2015-09-30 17:18 ` Mark Brown
2015-09-30 18:30 ` [PATCH v2 1/3] regmap: debugfs: use snprintf return value in regmap_reg_ranges_read_file() Rasmus Villemoes
2015-09-30 18:30 ` [PATCH v2 2/3] regmap: debugfs: use memcpy instead of snprintf Rasmus Villemoes
2015-09-30 18:30 ` [PATCH v2 3/3] regmap: debugfs: simplify regmap_reg_ranges_read_file() slightly Rasmus Villemoes
2015-09-29 18:14 ` [PATCH 1/2] regmap: debugfs: remove bogus check Mark Brown
2015-09-30 7:27 ` Rasmus Villemoes
2015-09-30 17:20 ` Mark Brown
2015-09-30 17:52 ` Rasmus Villemoes [this message]
2015-09-30 18:14 ` Mark Brown
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=87d1wzg5gl.fsf@rasmusvillemoes.dk \
--to=linux@rasmusvillemoes.dk \
--cc=broonie@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.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
all inboxes | Powered by JetHome®