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-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (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 AF4B51F4B4 for ; Sun, 13 Sep 2020 18:11:02 +0000 (UTC) Received: from localhost ([::1]:46664 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kHWSr-0003sT-Gc for normalperson@yhbt.net; Sun, 13 Sep 2020 14:11:01 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48438) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kHWSk-0003sJ-DA for bug-gnulib@gnu.org; Sun, 13 Sep 2020 14:10:54 -0400 Received: from mail-qk1-f193.google.com ([209.85.222.193]:37291) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kHWSi-0007vp-Kb for bug-gnulib@gnu.org; Sun, 13 Sep 2020 14:10:54 -0400 Received: by mail-qk1-f193.google.com with SMTP id 16so14842832qkf.4 for ; Sun, 13 Sep 2020 11:10:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=COR7sMTO3OsVoYmAMr9YQA9qvrbz7m9WqTZLkWaO2Bs=; b=sSt9h9sdURFIKbLw6A/hwc/YPkXtmdxE/U2ea5XrPQoWRhcoVFo8o0E+nUAtchMkJy qMbsBDCAqOM4uIuTn7EjkaLjUciCKtAGv4yPqCn8SoWPt3JGajRafDYK3GQ23Mfw2DQ6 ppVDqzdq4qUFd0tDo6JA6/9EO2NxmNiB6ooRYLKih9zqToxOLKvbnr4OWza17ofbXwcq ptohUKXaxnS6KE/wtpUDYH8ZgVW6BMbrVNYfnFC95v8RRWS/jdy8w/c0P8SGJ2Wx07P2 e1a7Cqw/v78ncmXcCE+XKA3Bf2s6In7tgoWPknhOYYkAJw4UwPzvYxVCvN7QDl/sUlgo 8y2Q== X-Gm-Message-State: AOAM533M0bBH17Vv6UO0FihTONoLOVP3scWepBEqDfsvRfq1h2OSdgob nQ0hFWZ+ySQOCcjTZ4Bf5nG4T6ZeShG37A== X-Google-Smtp-Source: ABdhPJxT0uD3IdFoRLk9ceVMipQuUyx35eXB1XtOcWOfaSY3qAdMhCC9L7jBrZdoMpqB7hiLeMko6Q== X-Received: by 2002:a37:a143:: with SMTP id k64mr10107782qke.266.1600020651135; Sun, 13 Sep 2020 11:10:51 -0700 (PDT) Received: from sigxcpu.benpfaff.org ([2600:1700:9920:6b60:41d5:b917:88cd:c38d]) by smtp.gmail.com with ESMTPSA id b43sm12165585qtk.84.2020.09.13.11.10.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 13 Sep 2020 11:10:50 -0700 (PDT) Date: Sun, 13 Sep 2020 11:10:47 -0700 From: Ben Pfaff To: Bruno Haible Subject: Re: [PATCH] getpass: Do not check for nonnull prompt argument in Win32 implementation. Message-ID: <20200913181047.GA3722105@sigxcpu.benpfaff.org> References: <20200912231030.2964126-1-blp@cs.stanford.edu> <2421457.Gk9qqdjgq8@omega> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2421457.Gk9qqdjgq8@omega> Received-SPF: pass client-ip=209.85.222.193; envelope-from=pfaffben@gmail.com; helo=mail-qk1-f193.google.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/13 14:10:51 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -13 X-Spam_score: -1.4 X-Spam_bar: - X-Spam_report: (-1.4 / 5.0 requ) BAYES_00=-1.9, FREEMAIL_FORGED_FROMDOMAIN=0.248, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Simon Josefsson , bug-gnulib@gnu.org, Jim Meyering Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" On Sun, Sep 13, 2020 at 10:32:27AM +0200, Bruno Haible wrote: > > The prompt parameter to getpass() is declared as nonnull (using a GCC > > nonnull attribute), but the implementation checks whether it is null in > > two places. GCC warns about this. This commit removes the checks > > GCC warnings ought to help us make the code more robust. Removing the > NULL check makes it less robust. > > The problem has already occurred a couple of times: > https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00050.html > https://lists.gnu.org/archive/html/bug-gnulib/2018-08/msg00116.html > https://lists.gnu.org/archive/html/bug-gnulib/2013-02/msg00060.html > https://lists.gnu.org/archive/html/bug-gnulib/2009-12/msg00173.html > > I would prefer that the same idiom gets used, that gets rid of the > warning without removing the NULL check at run time. Thanks. It is a little complicated because there are three implementations of getpass(), and two of them do not check the prompt: * The glibc implementation passes the prompt as %s to fprintf(). I guess that glibc will generally print "(null)", although I've seen GCC "optimize" similar things to fputs() in the past, which will dereference null. I guess I won't worry about it. * The POSIX implementation in gnulib passes the prompt to fputs() without checking for null. I guess we should fix it. I like the trick from one of your references about defining _GL_ARG_NONNULL to empty. That works OK here. So, how about like this? -8<--------------------------cut here-------------------------->8-- From: Ben Pfaff Date: Sat, 12 Sep 2020 15:54:36 -0700 Subject: [PATCH] getpass: Check for nonnull prompt argument while avoiding warnings. The prompt parameter to getpass() is declared as nonnull (using a GCC nonnull attribute). Gnulib contains two implementations of this function, one for POSIX, one for Windows. The Windows implementation checked for a nonnull prompt, which caused a GCC warning. This commit fixes that by avoiding the nonnull attribute when building getpass.c. The POSIX implementation did not check for a nonnull prompt. This commit increases the robustness by adding such a check. 2020-09-12 Ben Pfaff Check for nonnull prompt argument while avoiding warnings. * lib/getpass.c (_GL_ARG_NONNULL): Define to empty. (getpass) [!_WIN32]: Print prompt only if nonnull. Use __builtin_signbit* with clang. diff --git a/lib/getpass.c b/lib/getpass.c index 3b0552ec58..ca528fdc09 100644 --- a/lib/getpass.c +++ b/lib/getpass.c @@ -16,6 +16,9 @@ with this program; if not, see . */ #ifndef _LIBC +/* Don't use __attribute__ __nonnull__ in this compilation unit. Otherwise gcc + warns for the null checks on 'prompt' below. */ +# define _GL_ARG_NONNULL(params) # include #endif @@ -124,9 +127,12 @@ getpass (const char *prompt) } # endif - /* Write the prompt. */ - fputs_unlocked (prompt, out); - fflush_unlocked (out); + if (prompt) + { + /* Write the prompt. */ + fputs_unlocked (prompt, out); + fflush_unlocked (out); + } /* Read the password. */ nread = getline (&buf, &bufsize, in); -- 2.28.0