From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x112d.google.com (mail-yw1-x112d.google.com [IPv6:2607:f8b0:4864:20::112d]) by sourceware.org (Postfix) with ESMTPS id 7BDD0383980D for ; Thu, 19 May 2022 22:20:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7BDD0383980D Received: by mail-yw1-x112d.google.com with SMTP id 00721157ae682-2ff53d86abbso51491937b3.8 for ; Thu, 19 May 2022 15:20:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=WokuMRQfdm1F/5JuAZ0103mdS5mCm2Bqm3BM5kwegT0=; b=q0e+39wtyNF8kzuU/F7/RLi2ph3V8AXBcHuI4vUHNq83kB/BDodkbk5s4SMV9etKEA QrdLolhmalX/9Hz6zxzHYdQJb7QWjDFL/yH0DRHaUOjyZNLwvsdGlIbWSrnXTRWerHXG StVSUFyueXPqo0iBHFn1IIQVRVN+fBlPHpynGU25AXw6FWXf5dUpW99ekRq469EG+H7a xSdF0o9jYGcFK4aggNqPUGqM0RcsyctvNRfBVdxqnZoGOiyALE3EN/GiQbgBya5rbPyR IcR2V+p51+GvahcG0gIq46CWC/SwaTorKgcAvVtvXOUEM2V5WOyRdjvEKQLUB0U23oEA cQgg== X-Gm-Message-State: AOAM532d/yJzztasEBN9ft4HEcX2vvTtKCgueMUeOvHUF8njpqF+Lvn8 eUEVEIzqHwTBNwHjv/9Fo0ABR1DKEOGrziUwTyL93Xos X-Google-Smtp-Source: ABdhPJyxnWIXusNNl4fT1OeeefTymVtUqnBylVJYs/BHghF65up/oRDnVMDF/jC5NToreLXPYRtm4y/Q6iMR4dvq1uE= X-Received: by 2002:a81:c44b:0:b0:2d6:4726:ef4b with SMTP id s11-20020a81c44b000000b002d64726ef4bmr6940199ywj.184.1652998816918; Thu, 19 May 2022 15:20:16 -0700 (PDT) MIME-Version: 1.0 References: <20220414041231.926415-1-goldstein.w.n@gmail.com> <20220518172635.291119-1-goldstein.w.n@gmail.com> <20220518172635.291119-3-goldstein.w.n@gmail.com> <22a2afdb-a68c-f30c-e080-093e29c04d25@gotplt.org> <844b5851-38ca-3785-4f3a-5d219ea62d2d@gotplt.org> In-Reply-To: <844b5851-38ca-3785-4f3a-5d219ea62d2d@gotplt.org> From: Noah Goldstein Date: Thu, 19 May 2022 17:20:06 -0500 Message-ID: Subject: Re: [PATCH v10 3/6] nss: Add tests for the nss_hash in nss_hash.h To: Siddhesh Poyarekar Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Thu, 19 May 2022 22:20:20 -0000 On Thu, May 19, 2022 at 10:40 AM Siddhesh Poyarekar wrote: > > On 19/05/2022 20:39, Siddhesh Poyarekar wrote: > > On 18/05/2022 22:56, Noah Goldstein via Libc-alpha wrote: > >> If we want to further optimize the function tests are needed. > >> --- > >> nss/Makefile | 1 + > >> nss/nss_hash.c | 16 +++++++++ > >> nss/simple-nss-hash.h | 42 +++++++++++++++++++++++ > >> nss/tst-nss-hash.c | 80 +++++++++++++++++++++++++++++++++++++++++++ > >> 4 files changed, 139 insertions(+) > >> create mode 100644 nss/simple-nss-hash.h > >> create mode 100644 nss/tst-nss-hash.c > > > > LGTM. > > > > Reviewed-by: Siddhesh Poyarekar > > > >> > >> diff --git a/nss/Makefile b/nss/Makefile > >> index d8b06b44fb..a978e3927a 100644 > >> --- a/nss/Makefile > >> +++ b/nss/Makefile > >> @@ -62,6 +62,7 @@ tests := \ > >> test-digits-dots \ > >> test-netdb \ > >> tst-nss-getpwent \ > >> + tst-nss-hash \ > >> tst-nss-test1 \ > >> tst-nss-test2 \ > >> tst-nss-test4 \ > >> diff --git a/nss/nss_hash.c b/nss/nss_hash.c > >> index 27a348ea9b..f9e17d068a 100644 > >> --- a/nss/nss_hash.c > >> +++ b/nss/nss_hash.c > >> @@ -75,4 +75,20 @@ __nss_hash (const void *keyarg, size_t len) > >> return h; > >> } > >> +/* For testing/benchmarking purposes. */ > >> +static uint32_t > >> +__simple_nss_hash (const void *keyarg, size_t len) > >> +{ > >> + const unsigned char *key; > >> + size_t i; > >> + uint32_t h = 0; > >> + key = keyarg; > >> + > >> + for (i = 0; i < len; ++i) > >> + h = *key++ + 65599 * h; > >> + > >> + return h; > >> +} > >> + > >> + > > It just struck me (while reviewing 5/6) that this is duplicated in > simple-nss-hash.h below. Shouldn't it be one or the other? I know it's > "fixed" in 5/6 but it would be nice to restructure things so that the > tree builds at this point of the patchset too. Fixed V11. Forgot to fix in my last rebases :( > > >> libc_hidden_def (__nss_hash) > >> diff --git a/nss/simple-nss-hash.h b/nss/simple-nss-hash.h > >> new file mode 100644 > >> index 0000000000..47708972e7 > >> --- /dev/null > >> +++ b/nss/simple-nss-hash.h > >> @@ -0,0 +1,42 @@ > >> +/* __simple_nss_hash for testing nss_hash function > >> + Copyright (C) 2022 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 > >> + . */ > >> + > >> +#ifndef _SIMPLE_NSS_HASH_H > >> +#define _SIMPLE_NSS_HASH_H 1 > >> + > >> +#include > >> + > >> +/* For testing/benchmarking purposes. Real implementation in > >> + nss/nss_hash.c. */ > >> +static uint32_t > >> +__attribute__ ((unused)) > >> +__simple_nss_hash (const void *keyarg, size_t len) > >> +{ > >> + const unsigned char *key; > >> + size_t i; > >> + uint32_t h = 0; > >> + key = keyarg; > >> + > >> + for (i = 0; i < len; ++i) > >> + h = *key++ + 65599 * h; > >> + > >> + return h; > >> +} > >> + > >> + > >> +#endif /* simple-nss-hash.h */ > >> diff --git a/nss/tst-nss-hash.c b/nss/tst-nss-hash.c > >> new file mode 100644 > >> index 0000000000..5ec1f9b0c5 > >> --- /dev/null > >> +++ b/nss/tst-nss-hash.c > >> @@ -0,0 +1,80 @@ > >> +/* Test __nss_hash > >> + Copyright (C) 2022 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 > >> + > >> +uint32_t __nss_hash (const void *__key, size_t __length); > >> + > >> +static int > >> +do_fill_tests (size_t len, int fill) > >> +{ > >> + uint32_t expec, res; > >> + char buf[len]; > >> + memset (buf, fill, len); > >> + > >> + expec = __simple_nss_hash (buf, len); > >> + res = __nss_hash (buf, len); > >> + if (expec != res) > >> + FAIL_EXIT1 ("FAIL: fill(%d) (%zu), %x != %x\n", fill, len, expec, > >> res); > >> + > >> + return 0; > >> +} > >> + > >> +static int > >> +do_rand_tests (size_t len) > >> +{ > >> + uint32_t expec, res; > >> + size_t i; > >> + char buf[len]; > >> + for (i = 0; i < len; ++i) > >> + buf[i] = random (); > >> + > >> + expec = __simple_nss_hash (buf, len); > >> + res = __nss_hash (buf, len); > >> + if (expec != res) > >> + FAIL_EXIT1 ("FAIL: random (%zu), %x != %x\n", len, expec, res); > >> + > >> + return 0; > >> +} > >> + > >> +static int > >> +do_test (void) > >> +{ > >> + size_t i, j; > >> + for (i = 0; i < 100; ++i) > >> + { > >> + for (j = 0; j < 8192; ++j) > >> + { > >> + if (do_rand_tests (i)) > >> + return 1; > >> + > >> + if (do_fill_tests (i, -1) || do_fill_tests (i, 1) > >> + || do_fill_tests (i, 0x80) || do_fill_tests (i, 0x88)) > >> + return 1; > >> + } > >> + } > >> + return 0; > >> +} > >> + > >> +#include > > >