Re: svn commit: r329296 - head/sbin/devd
On Wed, 14 Feb 2018, Warner Losh wrote: On Wed, Feb 14, 2018 at 8:51 PM, Eitan Adler wrote: On 14 February 2018 at 19:48, Warner Losh wrote: On Wed, Feb 14, 2018 at 8:39 PM, Warner Losh wrote: On Feb 14, 2018 8:23 PM, "Eitan Adler" 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 ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r329296 - head/sbin/devd
On Wed, Feb 14, 2018 at 8:51 PM, Eitan Adler wrote: > On 14 February 2018 at 19:48, Warner Losh wrote: > > > > > > On Wed, Feb 14, 2018 at 8:39 PM, Warner Losh wrote: > >> > >> > >> > >> On Feb 14, 2018 8:23 PM, "Eitan Adler" wrote: > >> > >> Author: eadler > >> Date: Thu Feb 15 03:22:53 2018 > >> New Revision: 329296 > >> URL: https://svnweb.freebsd.org/changeset/base/329296 > >> > >> 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" > > 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... Warner ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r329296 - head/sbin/devd
On 14 February 2018 at 19:48, Warner Losh wrote: > > > On Wed, Feb 14, 2018 at 8:39 PM, Warner Losh wrote: >> >> >> >> On Feb 14, 2018 8:23 PM, "Eitan Adler" wrote: >> >> Author: eadler >> Date: Thu Feb 15 03:22:53 2018 >> New Revision: 329296 >> URL: https://svnweb.freebsd.org/changeset/base/329296 >> >> 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" 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. FTR I'd like to fix the declaration of select(2) anyways. -- Eitan Adler Source, Ports, Doc committer Bugmeister, Ports Security teams ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r329296 - head/sbin/devd
On Wed, Feb 14, 2018 at 8:39 PM, Warner Losh wrote:
>
>
> On Feb 14, 2018 8:23 PM, "Eitan Adler" wrote:
>
> Author: eadler
> Date: Thu Feb 15 03:22:53 2018
> New Revision: 329296
> URL: https://svnweb.freebsd.org/changeset/base/329296
>
> 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"
Warner
>
>
Warner
>
>
> Modified:
> head/sbin/devd/devd.cc
>
> Modified: head/sbin/devd/devd.cc
>
> ==
> --- head/sbin/devd/devd.cc Thu Feb 15 03:22:04 2018(r329295)
> +++ head/sbin/devd/devd.cc Thu Feb 15 03:22:53 2018(r329296)
> @@ -1021,7 +1021,7 @@ event_loop(void)
> tv.tv_usec = 0;
> FD_ZERO(&fds);
> FD_SET(fd, &fds);
> - rv = select(fd + 1, &fds, &fds, &fds, &tv);
> + rv = select(fd + 1, &fds, NULL, NULL, &tv);
> // No events -> we've processed all pending events
> if (rv == 0) {
> devdlog(LOG_DEBUG, "Calling daemon\n");
>
>
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r329296 - head/sbin/devd
On Feb 14, 2018 8:23 PM, "Eitan Adler" wrote:
Author: eadler
Date: Thu Feb 15 03:22:53 2018
New Revision: 329296
URL: https://svnweb.freebsd.org/changeset/base/329296
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.
Warner
Modified:
head/sbin/devd/devd.cc
Modified: head/sbin/devd/devd.cc
==
--- head/sbin/devd/devd.cc Thu Feb 15 03:22:04 2018(r329295)
+++ head/sbin/devd/devd.cc Thu Feb 15 03:22:53 2018(r329296)
@@ -1021,7 +1021,7 @@ event_loop(void)
tv.tv_usec = 0;
FD_ZERO(&fds);
FD_SET(fd, &fds);
- rv = select(fd + 1, &fds, &fds, &fds, &tv);
+ rv = select(fd + 1, &fds, NULL, NULL, &tv);
// No events -> we've processed all pending events
if (rv == 0) {
devdlog(LOG_DEBUG, "Calling daemon\n");
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
