X-Recipient:	archive-cygwin AT delorie DOT com
DKIM-Filter:	OpenDKIM Filter v2.11.0 sourceware.org 5F3C83861032
DKIM-Signature:	v=1; a=rsa-sha256; c=relaxed/relaxed; d=cygwin.com;
	s=default; t=1596461236;
	bh=ayP3D0l+mn/ZVjfd4PedF5M2NjDQ/kCjMBStB801LfI=;
	h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe:
	List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:
	From;
	b=YL8FmydrM76GbzSKa9n2o1iT3Vd+Wk0xt6HgaUJofA4+ciyPcDYrvEhqdOiwHjOZO
	J6p72oVZW4P9JFYqV7gddZxD30/w/oqJdD+E1h5dTTqNcmJhzuIASN/B7P2pj06/89
	ET2Zgv8cmrYSIC8NW0lLbKoFUv3+0yJ2/ju9Dr9Y=
X-Original-To:	cygwin AT cygwin DOT com
Delivered-To:	cygwin AT cygwin DOT com
DMARC-Filter:	OpenDMARC Filter v1.3.2 sourceware.org 966DF3858D35
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)
Subject:	Re: FD_{SET,ISSET,CLR} macros from sys/select.h triggerring gcc's
	-Wsign-conversion warnings
To:	cygwin AT cygwin DOT com
References:	<1f188b7e-6dc4-73af-e458-013760210469 AT gmail DOT com>
	<20200803104552 DOT GJ460314 AT calimero DOT vinschen DOT de>
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>
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 AT cygwin DOT com
X-Mailman-Version:	2.1.29
List-Id:	General Cygwin discussions and problem reports <cygwin.cygwin.com>
List-Unsubscribe:	<https://cygwin.com/mailman/options/cygwin>,
	<mailto:cygwin-request AT cygwin DOT com?subject=unsubscribe>
List-Archive:	<https://cygwin.com/pipermail/cygwin/>
List-Post:	<mailto:cygwin AT cygwin DOT com>
List-Help:	<mailto:cygwin-request AT cygwin DOT com?subject=help>
List-Subscribe:	<https://cygwin.com/mailman/listinfo/cygwin>,
	<mailto:cygwin-request AT cygwin DOT com?subject=subscribe>
From:	Petr Skocik via Cygwin <cygwin AT cygwin DOT com>
Reply-To:	Petr Skocik <pskocik AT gmail DOT com>
Errors-To:	cygwin-bounces AT cygwin DOT com
Sender:	"Cygwin" <cygwin-bounces AT cygwin DOT com>
X-MIME-Autoconverted:	from base64 to 8bit by delorie.com id 073DRhvQ003262

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 <sys/select.h>
>> 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
>

--
Problem reports:      https://cygwin.com/problems.html
FAQ:                  https://cygwin.com/faq/
Documentation:        https://cygwin.com/docs.html
Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple

- Raw text -