From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id 75C163858D32 for ; Wed, 12 Apr 2023 15:56:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 75C163858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=jguk.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=jguk.org Received: by mail-wr1-x42e.google.com with SMTP id v6so11393700wrv.8 for ; Wed, 12 Apr 2023 08:56:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jguk.org; s=google; t=1681315014; x=1683907014; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=VichOP4XCbh5Hqhm3jffW5WgvHEuq2y6p/wJEOl/uis=; b=CEatS7OycfVQ+nost70r/J+yqUAsT2KCey78zE0UZMOIhTMhY0Nvrj+YkywG+m9IzB q77CfeD62L7+OZOVc2WiGksAYf+a0PhnkDlFnzdPuuPtd51ZXrTznoUIqr69Tynf966w UFGeYFSbVOakgXcVwS+MJrRa960zwrSm6DSA4nvCv/2ptEOGgsuZLEy+RI8+kekw2Nkw 1/wAULa7FGLjSW1kuKie2QKiooSQOtQmQFJ68wvX5vCgG+XUVBI5np/+Ti92fuXRyHXB VcJb6SNAJuN7GNK69uew+JdQufwxxK9wCsZdjnYca9fnwye1jOcQ9kWzTXDfp7WEU9ge 2y0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1681315014; x=1683907014; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VichOP4XCbh5Hqhm3jffW5WgvHEuq2y6p/wJEOl/uis=; b=JTSyonk3daySJbJu2S9M7w0rdroBzMQ3buVwQ5RyNUfYDGv9mAjcfrT2KLfr00uXp0 GersyHNsyIb7PXDrkesKnzEML/rU8myRaavo85iWe8XpgqjEBmcKK7baDMW0JK4K8E2x 9Eguzd2G/uzmHVWduAAJEsJNZbD5Ser4pH7aMflcdapZuu5fhKPSOdXgfcqxjZwxKd7A 8D5oOlmFsufzN340PyhLT0ndLoU+TMFNLJ7Uza3GRbkqIwFtnUYoFqePHMd2Ua+617H0 CPDwKBSrf/RFOoDnXJR5F9qckwq8u7FT6TN/ROLJJ+OQQ2uPq4jIrO2k43vCUm3MfOeT l9CQ== X-Gm-Message-State: AAQBX9fa3T+M2HJrPjOGrT2l/WlnWYIMFSrPbQoyFx//v++AVaqAKfjF EO5ECwsojzNSDA5y1KhU4BSTtq6f7VES5WLuU4w= X-Google-Smtp-Source: AKy350afBlwuf8SV2MKVnvlTSTlrLJu0HjbH4zYQUAyxD2N56uesIrdrXexQHlGRtL6rwjxYPGhVcg== X-Received: by 2002:adf:d4c8:0:b0:2f0:91c3:16e7 with SMTP id w8-20020adfd4c8000000b002f091c316e7mr2447615wrk.28.1681315014095; Wed, 12 Apr 2023 08:56:54 -0700 (PDT) Received: from [192.168.0.12] (cpc87345-slou4-2-0-cust172.17-4.cable.virginm.net. [81.101.252.173]) by smtp.gmail.com with ESMTPSA id j1-20020a05600c488100b003ef5deb4188sm2735414wmp.17.2023.04.12.08.56.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 12 Apr 2023 08:56:53 -0700 (PDT) Message-ID: <514c11a4-405b-f7f3-9a67-0b6c10ad7740@jguk.org> Date: Wed, 12 Apr 2023 16:56:53 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: glibc misc/sys/cdefs.h nonull - typo in comment To: Adhemerval Zanella Netto , GNU C Library , Paul Eggert References: <25d0b6fa-7b45-3f8e-946a-ad3256e211a4@jguk.org> <0d99df74-fb83-1647-ca19-17d2229f0ae0@linaro.org> Content-Language: en-GB From: Jonny Grant In-Reply-To: <0d99df74-fb83-1647-ca19-17d2229f0ae0@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Adhemerval On 11/04/2023 14:39, Adhemerval Zanella Netto wrote: > > > On 11/04/23 06:09, Jonny Grant wrote: >> Hi >> >> There are two small changes I suggest if someone has a moment? >> >> misc/sys/cdefs.h nonull - typo in comment >> Should be "nonnull" > > This is already being fixed by c8ba52ab3350c334d (2.34). That's great news! I was interested to have a look through the history, and see the changes. Sorry I am bit new to sourceware git repo browser. May I ask, is there an easy way to look up that hex? It looks shorter than usual commit hex strings. The way I found the change was to use https://sourceware.org/git/?p=glibc.git "grep" search box with "c8ba52ab3350c334d" which found the ChangeLog with c8ba52ab3350c334d6e34b1439a4c0c1431351f3 Then I found another commit, and added to the URL https://sourceware.org/git/?p=glibc.git;a=commitdiff;h= https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=c8ba52ab3350c334d6e34b1439a4c0c1431351f3 >> Also >> >> /posix/glob.c has an unused macro >> # define _GL_ARG_NONNULL(params) >> >> Could that be removed? > > This definition is used by gnulib code, since it defined for !_LIBC, > Different than glibc, gnulib defines the function as: > > lib/glob.in.h > > 103 #if @GNULIB_GLOB@ > 104 # if @REPLACE_GLOB@ > 105 _GL_FUNCDECL_RPL (glob, int, (const char *_Restrict_ __pattern, int __flags, > 106 _gl_glob_errfunc_fn __errfunc, > 107 glob_t *_Restrict_ __pglob) > 108 _GL_ARG_NONNULL ((1))); > 109 _GL_CXXALIAS_RPL (glob, int, (const char *_Restrict_ __pattern, int __flags, > 110 _gl_glob_errfunc_fn __errfunc, > 111 glob_t *_Restrict_ __pglob)); > 112 # else > 113 # if !@HAVE_GLOB@ > 114 _GL_FUNCDECL_SYS (glob, int, (const char *_Restrict_ __pattern, int __flags, > 115 _gl_glob_errfunc_fn __errfunc, > 116 glob_t *_Restrict_ __pglob) > 117 _GL_ARG_NONNULL ((1))); > 118 # endif > 119 _GL_CXXALIAS_SYS (glob, int, (const char *_Restrict_ __pattern, int __flags, > 120 _gl_glob_errfunc_fn __errfunc, > 121 glob_t *_Restrict_ __pglob)); > 122 # endif > > Which then at the implementation is also check for pattern == NULL: > > lib/glob.c: > > 316 if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0) > 317 { > 318 __set_errno (EINVAL); > 319 return -1; > 320 } > > So the comment is right that compiler might indeed remove the test. > Different than gnulib, glibc prototype does not add the > __attribute__ ((nonnul)). > >> >> Seems _GL_ARG_NONNULL is only used in misc/error.c which again just compiles it out. So maybe it can be removed overall in both files? > > This was added as a sync with gnulib code by 888c679ba40, because it is > used in error_tail definition and glibc does not define it. So we can not > remove without also adjusting error_tail, and since the code is originally > from gnulib maybe it would be better to use __nonnull macro. Thank you for your reply with information. I'll read up a bit more next time before asking! Do you think it is risky to have the nonnull attribute in use in glibc? Some projects have abandoned attribute nonnull due to the GCC optimizer using the nonnull attribute to remove the runtime null pointer checks. https://gitlab.com/gnuwget/wget2/-/issues/200 Currently cdefs.h has /* The nonnull function attribute marks pointer parameters that must not be NULL. This has the name __nonnull in glibc, and __attribute_nonnull__ in files shared with Gnulib to avoid collision with a different __nonnull in DragonFlyBSD 5.9. */ Is it better to clarify this to be something like the following? /* The nonnull function attribute marks pointer parameters that the compiler's optimizer knows will never be NULL. This means NULL checks can be optimized out. This has the name __nonnull in glibc, and __attribute_nonnull__ in files shared with Gnulib to avoid collision with a different __nonnull in DragonFlyBSD 5.9. */ Kind regards, Jonny