From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by sourceware.org (Postfix) with ESMTPS id 966DF3858D35 for ; Mon, 3 Aug 2020 13:27:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 966DF3858D35 Received: by mail-wr1-x42c.google.com with SMTP id f1so33667184wro.2 for ; Mon, 03 Aug 2020 06:27:13 -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:message-id :disposition-notification-to:date:user-agent:mime-version :in-reply-to:content-transfer-encoding:content-language; bh=idpUjZtMjCyfRtbdQ+wGaMYaO1zXFXeeQADROQAfplA=; b=UL+hzbRLzSBUWxL8NiRhmWwM8/pYw2k9LWQUEuU8iX79TmpUBcECHz5iTJzfKQXTai Q5yo3z0PkaPVHjFMCQVfxBrr/jWTW1sczR5Kb85F4kHYKAmJVxDO5NF5A6GLvfB6TXoa TqP5GPDOnENf3ju9VEgWWKZo/ZpyUng8GvpPFIT7VqCIRvPoV66NzjEVuPu/vG5haCpz Jgi+EiI7lhnX6hGYQCXZ5PRhl9ioInreFvhqCOWVL8Cb4+hML/h4zj5QXa+QJK3W8Vf/ VDZveTqWVEFboozrFyCvbc55ce1VulJw8CeAVVy+fGEG09R28+2sYYETCImdA58A4sv8 fdtw== X-Gm-Message-State: AOAM533PQl/Af1qUAxhnq7n2sS3mrHSCytp/0wls3YRtTyWkRddGrPjP 6X4Rsu/rn4PfS439IbR8rO0QCtr0Rq8= X-Google-Smtp-Source: ABdhPJySw/H8LbPXoyYoAkCHJfCDtEA2C6vxx79O9h+NcM4qHQ6zN1FQ2CSx5v1NWfIhel/muDRKCg== X-Received: by 2002:a5d:6452:: with SMTP id d18mr14995666wrw.284.1596461232486; Mon, 03 Aug 2020 06:27:12 -0700 (PDT) Received: from [10.0.0.1] (91-139-110-250.customers.tmcz.cz. [91.139.110.250]) by smtp.gmail.com with ESMTPSA id c15sm23703589wme.23.2020.08.03.06.27.11 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 03 Aug 2020 06:27:11 -0700 (PDT) Subject: Re: FD_{SET,ISSET,CLR} macros from sys/select.h triggerring gcc's -Wsign-conversion warnings To: cygwin@cygwin.com References: <1f188b7e-6dc4-73af-e458-013760210469@gmail.com> <20200803104552.GJ460314@calimero.vinschen.de> From: Petr Skocik X-Tagtoolbar-Keys: D20200803152710918 Message-ID: <92f5646d-2a24-e3a4-f3e1-934416335e8d@gmail.com> Date: Mon, 3 Aug 2020 15:27:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20200803104552.GJ460314@calimero.vinschen.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GB_FREEMAIL_DISPTO, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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: cygwin@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: General Cygwin discussions and problem reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Aug 2020 13:27:15 -0000 Thanks for the patch, Corinna! I also reported it to Musl which has very similar code (I guess because of the same FreeBSD origins) and it's been pointed out to me by Alexander Monakov that the int-cast results in worse codegen because then the modulos/divisions need signed division instructions and can no longer be optimized to shifts and masks (glibc does the int-cast still). In light of that, if I were the maintainer, I think I'd just just make the macros into inline functions (POSIX says it's unspecified whether they're functions or macros) and cast the filedescriptor number to unsigned internally or something similar. #define __fdset_mask(n)    ((__fd_mask)1 << ((unsigned)(n) % _NFDBITS)) static inline void FD_CLR(int __fd, fd_set *__fdset) {     __fdset->__fds_bits[((unsigned)__fd)/_NFDBITS] &= ~__fdset_mask(__fd); } static inline int FD_ISSET(int __fd, fd_set *__fdset) {     return ((__fdset->__fds_bits[(unsigned)__fd/_NFDBITS] & __fdset_mask(n)) != 0); } static inline void FD_SET(int __fd, fd_set *__fdset) {     __fdset->__fds_bits[(unsigned)__fd/_NFDBITS] |= __fdset_mask(__fd); } //perhaps also: static inline void FD_ZERO(fd_set *__fdset) {     memset(__fdset,0,sizeof(*__fdset)); //shorter + should have the same codegen on gcc/clang } Alternatively, keeping them macros and casting the n argument to unsigned (rather than casting sizeof to int) would preserve the better codegen, but it would be a bit of a loss in terms of type safety. (The issue could well be attributed to gcc, too, because, arguably, (the implicit) -Wno-system-headers should have silenced it even without the explicit casts.) Anyway, sorry for all the fuss about such a negligible issue, but I guess fixing it (in whatever way) does make the lib a tiny bit more pleasant to use. Regards, Petr Skocik On 8/3/20 12:45 PM, Corinna Vinschen wrote: > Hi Petr, > > On Aug 2 20:08, Petr Skocik via Cygwin wrote: >> Example: >> >> #include >> void f(int X) >> { >>     fd_set set; >>     FD_ZERO(&set); >>     FD_SET(X,&set); >>     FD_CLR(X+1,&set); >>     (void)FD_ISSET(X+2,&set); >> } >> >> causes >> >> fds.c:7:2: warning: conversion to ‘long unsigned int’ from ‘int’ may >> change the sign of the result [-Wsign-conversion] >>   FD_SET(X,&set); >>   ^~~~~~ >> [...] >> on gcc with -Wconversion -Wsign-conversion. >> >> The problem is caused by the following macros: >> >>     #  define    NFDBITS    (sizeof (fd_mask) * 8)    /* bits per mask */ >>     #  define    FD_SET(n, p)    ((p)->fds_bits[(n)/NFDBITS] |= (1L << >> ((n) % NFDBITS))) >>     #  define    FD_CLR(n, p)    ((p)->fds_bits[(n)/NFDBITS] &= ~(1L << >> ((n) % NFDBITS))) >>     #  define    FD_ISSET(n, p)    ((p)->fds_bits[(n)/NFDBITS] & (1L << >> ((n) % NFDBITS))) >> >> int-casting the sizeof and using 1UL instead of 1L fixes the problem: > Thanks. I pushed a patch which will show up in the next Cygwin release: > https://sourceware.org/git/?p=newlib-cygwin.git;a=commitdiff;h=5717262b8ecf > > Funny enough, while the GLibc version of sys/select.h does not generate > these warnings, the same problem still exists in the upstream FreeBSD > code. > > For testing I uploaded a new developer snapshot to > https://cygwin.com/snapshots/ You only need to fetch the new > sys/select.h file. > > > Thanks, > Corinna >