From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.2 required=3.0 tests=AWL,BAYES_00,BODY_8BITS, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id AEED220248 for ; Wed, 20 Mar 2019 17:42:52 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=KDX0JaPdMjG9XSMr guUasP7W3Y3FD+kLDafLMyw6Mii2YaTQWQ06JWXyW/QGWMaJFawxgXIEVptP9Gy5 G8b4XabxdwH1bwKS2Z0bM3wqpZtT02Vl3/4wHnI4ldcW8ToO6v2J9BXz+Fe3k9Tb Z1NPoSfcaXPvj3ws4SL4bfAES0U= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=A9NsBwU5ROgu+0sjP8C2Xj Wt8Ro=; b=bkSIc+B547CQd8MvLlO9BWPyqY0xDl66aH+sYjavHfyJe2rxgiQQ42 dFJF22UrF4kEa2yuv8qJZ6XldJLCUFTCTaMsnCDP64Fj0h4xl7fsDM1QNRU5a//7 n0gs6r5i5k759wsaxMKLH38XpVpVyBhH1JnF0nsGRoTLFSbSSpQRc= Received: (qmail 102008 invoked by alias); 20 Mar 2019 17:42:49 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 101993 invoked by uid 89); 20 Mar 2019 17:42:49 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-ua1-f68.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:cc:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=M4H2eFgj1l0DZdC1rrBuVX9jmqkfMIY0J7bnbti5My4=; b=JkdF97bmrXoF+8XmeX2ySpfL84ABXfLPt91UkNCEXwQSvg6UT3NbM9nVuoaG5ZCvtN Tzd9JfiVejT0PlBY2gSY7dtTv85klMQ9eYypGCneQZqtfy4sDHnm301QT27ZsOAYzinV VPrIHLy42fqCI91rwNy+OvqD2kZT3U3FTeOWhLFp3FITNwVCFZ9swaIjR6Tn/UruPq+B 8B9XZu804snZnwFiPBY5xATwkwBzmVxzIFIWekuoEXiQneMUV5eLaquUM6hkVORxcIaq bLPB9YgMZ6lljEB18qeoHEbML4EwHzRVYN1qcsBR6QnpWeyvm+LWLmLKtomwf8s3+6x8 wg+g== To: Wilco Dijkstra Cc: nd , "libc-alpha@sourceware.org" References: <6ea276af-aa62-2bfe-ee6c-83c8eeaf283a@linaro.org> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH v2 4/6] Do not use HP_TIMING_NOW for random bits Message-ID: <4ba43d40-7b80-0e3f-ce20-a1db8c58dcd6@linaro.org> Date: Wed, 20 Mar 2019 14:42:43 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 20/03/2019 12:32, Wilco Dijkstra wrote: > Hi Adhemerval, > >> +# include >> +# define RANDOM_BITS(Var) ((Var) = random_bits ()) >> >> This define is not used (removed above). > >> I think we need to still define it if we eventually decide to sync it back >> to gnulib. > > Well this is the question - do we really need all this clutter just for gnulib? > It looks to me we should keep the code as clean as possible (so we don't > need any the !_LIBC code given these files are always in LIBC). I think we can address is in a subsequent patch, to either sync it with gnulib or just cleanup the non required parts. > >> I fact the new line should not be added, since random_time_bits should already >> get the random_bits() value. In any case I think we can remove random_time_bits >> altogether and just call RANDOM_BITS on value instead. > > Agreed. > >> And it seems 'value' is static by design, but I do agree there is no impeding >> reason to continue to do so. > > Indeed. > > --- > diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c > index 5217cb38e1..d062e4b82f 100644 > --- a/sysdeps/posix/tempname.c > +++ b/sysdeps/posix/tempname.c > @@ -73,6 +73,13 @@ >  #ifdef _LIBC >  # include >  # define RANDOM_BITS(Var) ((Var) = random_bits ()) > +# else > +# define RANDOM_BITS(Var) \ > +    {                                                                         \ > +      struct timeval tv;                                                      \ > +      __gettimeofday (&tv, NULL);                                             \ > +      (Var) = ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec;                      \ > +    } >  #endif >   > I don't see the point of this, especially using gettimeofday. If we want to export > these changes to gnulib, we could just add random_bits to gnulib. >From gnulib doc doc/posix-functions/clock_gettime.texi, clock_gettime is missing on some platforms (OS X 10.11, Minix 3.1.8, IRIX 5.3, Solaris 2.4, mingw, MSVC 14, Interix 3.5, BeOS) and implements gettime module using gettimeofday. No sure how gnulib would want to to implement it, or if it is willing to add another module to provide random_bits. In any case I think best course of action to still use the RANDOM_BITS with the fallback case using gettimeofday (even if it is not actively used on glibc) and once it is upstream I will send a patch to cleanup this and check with Paul Eggert what gnulib would do (deviate from glibc or implement something random_bits). > > > -  value += random_time_bits ^ __getpid (); > -  value += random_bits () ^ __getpid (); > +  RANDOM_BITS (value); > +  value ^= __getpid (); > +  /* Shuffle the lower bits to minimize the pid bias due low maximum value.  */ > +  value = (value << 24) | (value >> 8); > > random_bits already does that shuffle, so doing it again doesn't help. It's better > to avoid the aliasing of getpid with the random bits, eg. value ^= __get_pid << 32 > so we end up with more than 32 random bits. Alright I change it locally to ^= __getpid () << 32;