Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 18 Oct 2020 12:51:35 -0700
From:      Xin Li <delphij@delphij.net>
To:        Konstantin Belousov <kostikbel@gmail.com>, Xin LI <delphij@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r366781 - in head: include lib/libc/stdlib
Message-ID:  <592395b1-3985-5416-a06a-44335321799f@delphij.net>
In-Reply-To: <20201017065326.GY2643@kib.kiev.ua>
References:  <202010170414.09H4EdBF097521@repo.freebsd.org> <20201017065326.GY2643@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi,

Thanks very much for the feedback.  I have created a new change for
review at https://reviews.freebsd.org/D26845 , could you please take a
look and let me know if it's satisfactory?

On 10/16/20 11:53 PM, Konstantin Belousov wrote:
[...]>> +int	 ptsname_r(int, char *, size_t);
> This declaration appears in the __XSI_VISIBLE block, but I do not see the
> function description in the IEEE Std 1003.1™-2017 text (base issue 7).
> 
> Review mentioned that the function is scheduled for issue 8, but shouldn't
> it put under BSD_VISIBLE until specification is finalized ?

Good point, fixed in the proposed change.

[...]
>> Modified: head/lib/libc/stdlib/Symbol.map
>> ==============================================================================
>> --- head/lib/libc/stdlib/Symbol.map	Sat Oct 17 01:06:04 2020	(r366780)
>> +++ head/lib/libc/stdlib/Symbol.map	Sat Oct 17 04:14:38 2020	(r366781)
>> @@ -125,6 +125,7 @@ FBSD_1.6 {
>>  	qsort_s;
>>  	rand;
>>  	srand;
>> +	ptsname_r;
> This is unsorted now.

Fixed in the proposed change.

[...]
>> +{
>> +	static char pt_slave[sizeof _PATH_DEV + SPECNAMELEN];
> We usually write sizeof(_PATH_DEV).

This was from previous code so I leave it as-is, but fixed in the
proposed change.

>> +
>> +	if (ptsname_r(fildes, pt_slave, sizeof(pt_slave)) == 0)
> Since ptsname_r is non-standard and exported, userspace is allowed to
> interpose the symbol, which would break ptsname().

Thanks for pointing it out -- I should have paid more attention here.
Fixed in the proposed change.

>> +		return (pt_slave);
>> +	else
> 'else' is redundand.

Removed else and used a straight return instead.

Cheers,



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?592395b1-3985-5416-a06a-44335321799f>