All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Keno Fischer <keno@juliacomputing.com>
Cc: Markus Armbruster <armbru@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Greg Kurz <groug@kaod.org>, Gerd Hoffmann <kraxel@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4] cutils: Provide strchrnul
Date: Tue, 12 Jun 2018 09:07:42 +0200	[thread overview]
Message-ID: <87muw0wkht.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <CABV8kRwu7=FaYtZhOBCXZBbtmDXQgJpYCU3LGJToc_sk=6n9vA@mail.gmail.com> (Keno Fischer's message of "Mon, 11 Jun 2018 16:44:25 -0400")

Keno Fischer <keno@juliacomputing.com> writes:

>> Suggest return strchrnul("Hello World", 'W') != 6, to avoid worries
>> about a sufficiently smart compilers optimizing out a call that would
>> otherwise fail to link, say because headers don't match libraries.
>
> I'm happy to do that, but then again, a sufficiently smart compiler might
> constant fold this call entirely, so to be completely safe maybe we need
>
> extern char *haystack;
> extern char needle;
> int main(void) { return strchrnul(haystack, needle) != 6; }
>
> Though frankly if you're in a position for this to be a problem, you've
> got bigger problems. Happy to change this though.

Yes, please.  You even get to pick your favorite number for the right
hand side of the comparison ;)

>> Should this be named HAVE_STRCHRNUL?  It's how it would be named with
>> Autoconf...
>
> Ok, I will rename this.
>
>>> +const char *qemu_strchrnul(const char *s, int c)
>>> +{
>>> +    const char *e = strchr(s, c);
>>> +    if (!e) {
>>> +        e = s + strlen(s);
>>> +    }
>>> +    return e;
>>
>> Stupidest solution that could possibly work.  Okay :)
>
> Well, it's the pattern that was used everywhere in place of this function,
> so certainly from a commit factoring this seemed like the most sensible
> thing to do.
>
>> How did you find the spots to convert to strchrnul()?
>
> I audited uses of `strchr` and checked for whether they were really doing
> `strchrnul` (plus the one use case in code that used to only ever be compiled
> on Linux).

Sounds good.  Thanks!

      reply	other threads:[~2018-06-12  7:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-10 18:02 [Qemu-devel] [PATCH v4] cutils: Provide strchrnul Keno Fischer
2018-06-11  7:56 ` Markus Armbruster
2018-06-11  8:52   ` Peter Maydell
2018-06-11  9:33     ` Dr. David Alan Gilbert
2018-06-11  9:38     ` Daniel P. Berrangé
2018-06-11 10:11       ` Peter Maydell
2018-06-13  5:50     ` Markus Armbruster
2018-06-11 20:44   ` Keno Fischer
2018-06-12  7:07     ` Markus Armbruster [this message]

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=87muw0wkht.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=groug@kaod.org \
    --cc=keno@juliacomputing.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.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 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.