svn commit: r329296 - head/sbin/devd

Bruce Evans brde at optusnet.com.au
Thu Feb 15 12:23:07 UTC 2018


On Wed, 14 Feb 2018, Warner Losh wrote:

> On Wed, Feb 14, 2018 at 8:51 PM, Eitan Adler <eadler at freebsd.org> wrote:
>
>> On 14 February 2018 at 19:48, Warner Losh <imp at bsdimp.com> wrote:
>>>
>>> On Wed, Feb 14, 2018 at 8:39 PM, Warner Losh <imp at bsdimp.com> wrote:
>>>>
>>>> On Feb 14, 2018 8:23 PM, "Eitan Adler" <eadler at freebsd.org> wrote:
>>>> Log:
>>>>   devd: don't pass &fds in useless parameters to select(2)
>>>>
>>>>   select(2) should be declared as restrict. In addition the only fd in
>>>>   the fdset is open O_RDONLY, and it's not a socket that can provide OOB
>>>>   notifications,
>>>>
>>>>   Reviewed by:  ian, imp, vangyzen
>>>>
>>>> Don't put my name on this. I specifically and clearly objected to the
>>>> change anf tld yoy not to do it.
>>>
>>> Stupid phone...
>>>
>>> I specifically objected to this change. I said not to make it because it
>>> wasn't necessary. You did it any way. Don't put "reviewed by" for that.
>> Put
>>> "objected to but I did it anyway by: imp"

It is worse than unnecessary.  It breaks at least "any" in the comment
before it.  "any" means read, write or exeptional descriptors, but now
only read descriptors are checked.

>> hrm.. rereading the thread I think I missed your original email. Only
>> comment I saw was "poll is a better interface". Sorry for mis-stating
>> your opinion.
>
> Yea. It was more of a "don't change it, since it's fine now" and then the
> "poll" comment was "better to just change to a better interface."
>
>> FTR I'd like to fix the declaration of select(2) anyways.
>
> That's unlikely to end well... I wish you luck... The pattern I used in
> devd for select was nearly universal a decade ago... Maybe things have
> changed...

It can't be nearly universal except possibly for the very special check
where you don't care which events occurred, just whether there was one,
and also don't wait for any events, but just check for one in a racy
way.

Bruce


More information about the svn-src-all mailing list