From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by sourceware.org (Postfix) with ESMTP id 721323861815 for ; Tue, 21 Jul 2020 03:28:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 721323861815 Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-52-JvWKpVgXPuqoWBREwujqkQ-1; Mon, 20 Jul 2020 23:28:10 -0400 X-MC-Unique: JvWKpVgXPuqoWBREwujqkQ-1 Received: by mail-qt1-f199.google.com with SMTP id l53so13389003qtl.10 for ; Mon, 20 Jul 2020 20:28:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=QZvRf7y21XlcP00MhL+80VmU9GMgdtKA5p57DheMi3E=; b=EjgZxUfto9rVYnPT4Br6MwaQUNSvc1Tq5NP2GsXUxelNHnnG39VVENVrB3jn8wsK0Q ymJCyI2PRd24F9XIGVwVhZwKUh+b6T7mDnijS9+mKeHmrJWYqqcJX0VVVx4N2RwIJ8YU k5v0cOqlXNtcIBLeLPdgO7eb6iKbcGQ706GAcmp138XBzgUPUgME25sk2VhBqJDurPwQ rAaoWv8RH/De2Tw6ljAIH0zHsH1qtiE40JHX9dRJW6bSYFIs1Kz9NHf4KyL5wdv4nXjk FthVexxzVQkMNgpkkGyc3IK3V/uzsCM7YGdi55hh6ZNHWZVniRC0/MDZ9jmGyWHVwSt5 LgLQ== X-Gm-Message-State: AOAM532LJ6Ve65M5llu5cfYaSgKrfiu8RFfLXWA007DqcE+eM8dlMVZp cF4T5215qsZtAnriIOdpdGjXOrh2GF5E46h+o7L0P8suO/zqZgmP04Apz1JWeG7I9sNwoTuOenn FiXqd8S32cvOYp68rX8Xz X-Received: by 2002:ac8:47d0:: with SMTP id d16mr27187707qtr.349.1595302089304; Mon, 20 Jul 2020 20:28:09 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx2FBHHOQyESJnZeNj9a6eRnoHh/JwGsv5GkQ1jr0OMDi0VAsUGk/wC2INFpJgtH1mRiBvvrw== X-Received: by 2002:ac8:47d0:: with SMTP id d16mr27187697qtr.349.1595302088986; Mon, 20 Jul 2020 20:28:08 -0700 (PDT) Received: from [192.168.1.4] (198-84-170-103.cpe.teksavvy.com. [198.84.170.103]) by smtp.gmail.com with ESMTPSA id 16sm1377338qkv.48.2020.07.20.20.28.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 20 Jul 2020 20:28:08 -0700 (PDT) Subject: Re: [PATCH 08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338) To: Florian Weimer , libc-alpha@sourceware.org References: From: Carlos O'Donell Organization: Red Hat Message-ID: Date: Mon, 20 Jul 2020 23:28:07 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Jul 2020 03:28:14 -0000 On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote: Huh, there isn't a man page for fgetsgent_r :} OK for 2.32. Good cleanup and test. Implement with __nss_fgetent_r and because we can use a FILE* we can test it with an arbitrary file. Tested-by: Carlos O'Donell Reviewed-by: Carlos O'Donell > --- > gshadow/Makefile | 2 +- > gshadow/fgetsgent_r.c | 41 ++------ > gshadow/tst-fgetsgent_r.c | 191 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 198 insertions(+), 36 deletions(-) > create mode 100644 gshadow/tst-fgetsgent_r.c > > diff --git a/gshadow/Makefile b/gshadow/Makefile > index 089aa6125f..32faf7a534 100644 > --- a/gshadow/Makefile > +++ b/gshadow/Makefile > @@ -26,7 +26,7 @@ headers = gshadow.h > routines = getsgent getsgnam sgetsgent fgetsgent putsgent \ > getsgent_r getsgnam_r sgetsgent_r fgetsgent_r > > -tests = tst-gshadow tst-putsgent > +tests = tst-gshadow tst-putsgent tst-fgetsgent_r > > CFLAGS-getsgent_r.c += -fexceptions > CFLAGS-getsgent.c += -fexceptions > diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c > index a7a1860c76..218206b4ac 100644 > --- a/gshadow/fgetsgent_r.c > +++ b/gshadow/fgetsgent_r.c > @@ -36,40 +36,11 @@ int > __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen, > struct sgrp **result) > { > - char *p; > - > - _IO_flockfile (stream); > - do > - { > - buffer[buflen - 1] = '\xff'; > - p = fgets_unlocked (buffer, buflen, stream); > - if (p == NULL && feof_unlocked (stream)) > - { > - _IO_funlockfile (stream); > - *result = NULL; > - __set_errno (ENOENT); > - return errno; > - } > - if (p == NULL || buffer[buflen - 1] != '\xff') > - { > - _IO_funlockfile (stream); > - *result = NULL; > - __set_errno (ERANGE); > - return errno; > - } > - > - /* Skip leading blanks. */ > - while (isspace (*p)) > - ++p; > - } while (*p == '\0' || *p == '#' /* Ignore empty and comment lines. */ > - /* Parse the line. If it is invalid, loop to > - get the next line of the file to parse. */ > - || ! parse_line (buffer, (void *) resbuf, (void *) buffer, buflen, > - &errno)); > - > - _IO_funlockfile (stream); > - > - *result = resbuf; > - return 0; > + int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line); > + if (ret == 0) > + *result = resbuf; > + else > + *result = NULL; > + return ret; > } > weak_alias (__fgetsgent_r, fgetsgent_r) > diff --git a/gshadow/tst-fgetsgent_r.c b/gshadow/tst-fgetsgent_r.c > new file mode 100644 > index 0000000000..780409afa9 > --- /dev/null > +++ b/gshadow/tst-fgetsgent_r.c > @@ -0,0 +1,191 @@ > +/* Test for fgetsgent_r and buffer sizes. > + Copyright (C) 2020 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* Turn a parsed struct back into a line string. The returned string > + should be freed. */ > +static char * > +format_ent (const struct sgrp *e) > +{ > + struct xmemstream stream; > + xopen_memstream (&stream); > + TEST_COMPARE (putsgent (e, stream.out), 0); > + xfclose_memstream (&stream); > + return stream.buffer; > +} > + > +/* An entry in the input file along with the expected output. */ > +struct input > +{ > + const char *line; /* Line in the file. */ > + const char *expected; /* Expected output. NULL if skipped. */ > +}; > + > +const struct input inputs[] = > + { > + /* Regular entries. */ > + { "g1:x1::\n", "g1:x1::\n" }, > + { "g2:x2:a1:\n", "g2:x2:a1:\n" }, > + { "g3:x3:a2:u1\n", "g3:x3:a2:u1\n" }, > + { "g4:x4:a3,a4:u2,u3,u4\n", "g4:x4:a3,a4:u2,u3,u4\n" }, > + > + /* Comments and empty lines. */ > + { "\n", NULL }, > + { " \n", NULL }, > + { "\t\n", NULL }, > + { "#g:x::\n", NULL }, > + { " #g:x::\n", NULL }, > + { "\t#g:x::\n", NULL }, > + { " \t#g:x::\n", NULL }, > + > + /* Marker for synchronization. */ > + { "g5:x5::\n", "g5:x5::\n" }, > + > + /* Leading whitespace. */ > + { " g6:x6::\n", "g6:x6::\n" }, > + { "\tg7:x7::\n", "g7:x7::\n" }, > + > + /* This is expected to trigger buffer exhaustion during parsing > + (bug 20338). */ > + { > + "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n", > + "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n", > + }, > + { > + "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n", > + "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n", > + }, > + }; > + > +/* Writes the test data to a temporary file and returns its name. The > + returned pointer should be freed. */ > +static char * > +create_test_file (void) > +{ > + char *path; > + int fd = create_temp_file ("tst-fgetsgent_r-", &path); > + FILE *fp = fdopen (fd, "w"); > + TEST_VERIFY_EXIT (fp != NULL); > + > + for (size_t i = 0; i < array_length (inputs); ++i) > + fputs (inputs[i].line, fp); > + > + xfclose (fp); > + return path; > +} > + > +/* Read the test file with the indicated start buffer size. Return > + true if the buffer size had to be increased during reading. */ > +static bool > +run_test (const char *path, size_t buffer_size) > +{ > + bool resized = false; > + FILE *fp = xfopen (path, "r"); > + > + /* This avoids repeated lseek system calls (bug 26257). */ > + TEST_COMPARE (fseeko64 (fp, 0, SEEK_SET), 0); > + > + size_t i = 0; > + while (true) > + { > + /* Skip over unused expected entries. */ > + while (i < array_length (inputs) && inputs[i].expected == NULL) > + ++i; > + > + /* Store the data on the heap, to help valgrind to detect > + invalid accesses. */ > + struct sgrp *result_storage = xmalloc (sizeof (*result_storage)); > + char *buffer = xmalloc (buffer_size); > + struct sgrp **result_pointer_storage > + = xmalloc (sizeof (*result_pointer_storage)); > + > + int ret = fgetsgent_r (fp, result_storage, buffer, buffer_size, > + result_pointer_storage); > + if (ret == 0) > + { > + TEST_VERIFY (*result_pointer_storage != NULL); > + TEST_VERIFY (i < array_length (inputs)); > + if (*result_pointer_storage != NULL > + && i < array_length (inputs)) > + { > + char * actual = format_ent (*result_pointer_storage); > + TEST_COMPARE_STRING (inputs[i].expected, actual); > + free (actual); > + ++i; > + } > + else > + break; > + } > + else > + { > + TEST_VERIFY (*result_pointer_storage == NULL); > + TEST_COMPARE (ret, errno); > + > + if (ret == ENOENT) > + { > + TEST_COMPARE (i, array_length (inputs)); > + free (result_pointer_storage); > + free (buffer); > + free (result_storage); > + break; > + } > + else if (ret == ERANGE) > + { > + resized = true; > + ++buffer_size; > + } > + else > + FAIL_EXIT1 ("read failure: %m"); > + } > + > + free (result_pointer_storage); > + free (buffer); > + free (result_storage); > + } > + > + return resized; > +} > + > +static int > +do_test (void) > +{ > + char *path = create_test_file (); > + > + for (size_t buffer_size = 3; ; ++buffer_size) > + { > + bool resized = run_test (path, buffer_size); > + if (!resized) > + break; > + } > + > + free (path); > + > + return 0; > +} > + > +#include > -- Cheers, Carlos.