From: Paul Jackson <pj@sgi.com>
To: Horst von Brand <vonbrand@inf.utfsm.cl>
Cc: jesper.juhl@gmail.com, linux-kernel@vger.kernel.org, jgarzik@pobox.com
Subject: Re: [PATCH 3/3] exterminate strtok - usr/gen_init_cpio.c
Date: Wed, 24 Aug 2005 21:46:34 -0700 [thread overview]
Message-ID: <20050824214634.6008be53.pj@sgi.com> (raw)
In-Reply-To: <200508242106.j7OL61QK010645@laptop11.inf.utfsm.cl>
Horst wrote:
> > - if ('\n' == *type) {
> > + if (!*type || '\n' == *type) {
>
> Redundant. If *type == '\n', it is certainly != 0.
No - I don't think redundant, at least not this change in isolation.
Perhaps redundant in light of subsequent code lines, as Jesper notes in
his followup.
But it is confusing to read - poor and inconsistent choice of code
phrasing.
If the patch had read as:
- if (*type == '\n') {
+ if (*type == '\n' || *type == '\0') {
then it would be clearer to the reader in my view. A check for newline
is changed to a check for newline or nul-byte.
(Yes - I recognize that one is not given the freedom to change the
-old- lines in a patch for the sake of clarity ;).
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@sgi.com> 1.925.600.0401
next prev parent reply other threads:[~2005-08-25 4:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-08-24 19:08 [PATCH 3/3] exterminate strtok - usr/gen_init_cpio.c Jesper Juhl
2005-08-24 20:12 ` Brian Gerst
2005-08-24 20:14 ` Jeff Garzik
2005-08-24 20:31 ` Jesper Juhl
2005-08-24 20:39 ` Brian Gerst
2005-08-24 21:14 ` Jesper Juhl
2005-08-26 15:31 ` Horst von Brand
2005-08-24 21:06 ` Horst von Brand
2005-08-24 21:15 ` Jesper Juhl
2005-08-25 4:46 ` Paul Jackson [this message]
2005-08-25 5:00 ` Sam Ravnborg
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=20050824214634.6008be53.pj@sgi.com \
--to=pj@sgi.com \
--cc=jesper.juhl@gmail.com \
--cc=jgarzik@pobox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=vonbrand@inf.utfsm.cl \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.