public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: DJ Delorie <dj@redhat.com>, libc-alpha@sourceware.org
Subject: Re: [bz 22342] patch: fix netgroup cache keys
Date: Thu, 01 Mar 2018 23:12:00 -0000	[thread overview]
Message-ID: <97cab0a7-7ea0-ab38-1fc6-ef6a007c0378@redhat.com> (raw)
In-Reply-To: <xnh8pzsayx.fsf@greed.delorie.com>

On 03/01/2018 02:11 PM, DJ Delorie wrote:
> Functionality tested on RHEL 7.  Regression tested on Fedora 26.
> 
> 	[BZ #22342]
> 	* nscd/netgroupcache.c (addinnetgrX): Include trailing NUL in
> 	key value.
> 
> diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
> index b832c9315f..2f187b208c 100644
> --- a/nscd/netgroupcache.c
> +++ b/nscd/netgroupcache.c
> @@ -480,7 +480,7 @@ addinnetgrX (struct database_dyn *db, int fd, request_header *req,
>  {
>    const char *group = key;
>    key = (char *) rawmemchr (key, '\0') + 1;
> -  size_t group_len = key - group - 1;
> +  size_t group_len = key - group;
>    const char *host = *key++ ? key : NULL;
>    if (host != NULL)
>      key = (char *) rawmemchr (key, '\0') + 1;
> 

Why is this correct?

A good submission includes a justification that the
fix is logically what is required.

We do not want to paper over a problem just by testing
that a given change makes things better.

Do we understand what the problem is?

The bug report appears to make the claim that there are
two paths in the code, one which adds to the cache with
the null included in the length, and another which doesn't,
which obviously results in a cache miss.

Is that the final case we are fixing here? Can you describe
this a bit more for the commit message?

Please post a v2 with the commit message you intend to use
please.

-- 
Cheers,
Carlos.

  reply	other threads:[~2018-03-01 23:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-01 22:11 DJ Delorie
2018-03-01 23:12 ` Carlos O'Donell [this message]
2018-03-01 23:21   ` DJ Delorie
2018-03-02  0:17     ` Carlos O'Donell
2018-03-02  2:28       ` [bz 22342] patch V2: " DJ Delorie
2018-03-02  3:13         ` Carlos O'Donell
2018-03-02  4:25           ` DJ Delorie
2018-03-02 17:20             ` Joseph Myers
2018-03-02 18:11               ` DJ Delorie

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=97cab0a7-7ea0-ab38-1fc6-ef6a007c0378@redhat.com \
    --to=carlos@redhat.com \
    --cc=dj@redhat.com \
    --cc=libc-alpha@sourceware.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 read-only IMAP folder(s) and NNTP newsgroup(s).