Re: CVS commit: src/sys/dev/scsipi
Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Wed May 13 02:35:25 UTC 2009 > > Modified Files: > src/sys/dev/scsipi: scsipiconf.h > > Log Message: > sprinkle #ifdef _KERNEL to make scsictl compile. > Ah, the device_t change exposed something that shouldn't be visible in userspace, it seems. Thanks for fixing it and sorry for build breakage. I should have also compile-tested userland of one arch at least, not just many kernels. Christoph
Re: CVS commit: src/sys/dev/dec
Christoph Egger writes: > Izumi Tsutsui wrote: >>> Now, of course, that doesn't come as a surprise. Some people just never >>> learn or listen. Or apologise. >> >> Honestly, I'd like to see proper fixes for output of >> "grep -r 'memcpy( ' sys/arch" , >> before another set of mechanical changes. > > I broke it, so I will fix it. > Do you want to review the patch ? If you compare the binary file output, there will be no need for review because you won't be guessing that stuff hasn't changed, you will *know*. I don't think you should commit any more cosmetic changes if you aren't willing to do that sort of check. If you script it, it isn't even terribly much effort. Perry -- Perry E. Metzgerpe...@piermont.com
Re: CVS commit: src/crypto/external/bsd/netpgp/dist
"Alistair G. Crooks" writes: > + allow a choice of hash algorithms for the signature digest (rather > than hardcoding SHA1 - it is looking as though collisions are easier > to manufacture based on recent findings) > + move default signature RSA hash algorithm to SHA256 (from SHA1). This is > passed as a string parameter from the high-level interface. We'll > revisit this later after a good way to specify the algorithm has been > found. I presume this isn't in the man page because you're waiting on the final method to handle it? Perry
re: CVS commit: src/sys/netinet
Quentin Garnier wrote: > On Tue, May 12, 2009 at 09:48:42PM +, Elad Efrat wrote: >> Module Name: src >> Committed By: elad >> Date: Tue May 12 21:48:42 UTC 2009 >> >> Modified Files: >> src/sys/netinet: ip_carp.c >> >> Log Message: >> Fix inverted permissions check. >> - if ((l == NULL) || (error = kauth_authorize_network(l->l_cred, >> + if ((l != NULL) || (error = kauth_authorize_network(l->l_cred, > > That can't be right. If l is NULL, then it should be dereferenced? Right, fixed, thanks! actually, you can remove all the check for l vs NULL in this file. they are useless since yamt-idlelwp. .mrg.
Re: CVS commit: src/sys/netinet
Quentin Garnier wrote: On Tue, May 12, 2009 at 09:48:42PM +, Elad Efrat wrote: Module Name:src Committed By: elad Date: Tue May 12 21:48:42 UTC 2009 Modified Files: src/sys/netinet: ip_carp.c Log Message: Fix inverted permissions check. - if ((l == NULL) || (error = kauth_authorize_network(l->l_cred, + if ((l != NULL) || (error = kauth_authorize_network(l->l_cred, That can't be right. If l is NULL, then it should be dereferenced? Right, fixed, thanks! -e.
Re: CVS commit: src/sys/netinet
On Tue, May 12, 2009 at 09:48:42PM +, Elad Efrat wrote: > Module Name: src > Committed By: elad > Date: Tue May 12 21:48:42 UTC 2009 > > Modified Files: > src/sys/netinet: ip_carp.c > > Log Message: > Fix inverted permissions check. > - if ((l == NULL) || (error = kauth_authorize_network(l->l_cred, > + if ((l != NULL) || (error = kauth_authorize_network(l->l_cred, That can't be right. If l is NULL, then it should be dereferenced? Is there a contest for tree breakage these days? -- Quentin Garnier - c...@cubidou.net - c...@netbsd.org "See the look on my face from staying too long in one place [...] every time the morning breaks I know I'm closer to falling" KT Tunstall, Saving My Face, Drastic Fantastic, 2007. pgp06wENPfl9k.pgp Description: PGP signature
Re: CVS commit: src/sys/dev/dec
I wrote: > > Maybe ahc(4) was the only one that was broken. The point is that you > > could have found this by comparing the object files. > > Although ahc(4) has many quirks (to share sources among *BSD/Linux), > only aic7xxx_osm.c belongs to ahc(4) and it doesn't have any bad cast, > per output of grep adapt_dev sys/dev/ic/*. Ah, in ahc_detach(), there is one misleading cast against device_t self to get softc. In this case, maybe ahc_detach() should be changed to take softc... (ahc_cardbus.c passes (void *)ahc to ahc_detach()'s device_t arg. Oh well.) Maybe we should discuss which arg should such MI detach functions take, since detach functions in frontends take device_t... --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/dec
> > Honestly, I'd like to see proper fixes for output of > > "grep -r 'memcpy( ' sys/arch" , > > before another set of mechanical changes. > > I broke it, so I will fix it. > Do you want to review the patch ? Well, it isn't functional changes at all. Can you check it yourself before calling for review? (BTW, didn't you even try cvs diff on each your changes?) I did the following checks on my previous memcpy( fixes in src/sys except src/sys/arch: - check all lines if they were caused by the mechanical replacement (some sources had had whitespaces) - check if each bcopy() should actually be converted or not - cmp(1) checks against binaries compiled in i386/conf/ALL and some several arch (but not all) --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/dec
Izumi Tsutsui wrote: >> Now, of course, that doesn't come as a surprise. Some people just never >> learn or listen. Or apologise. > > Honestly, I'd like to see proper fixes for output of > "grep -r 'memcpy( ' sys/arch" , > before another set of mechanical changes. I broke it, so I will fix it. Do you want to review the patch ? Christoph
Re: CVS commit: src/sys/dev/dec
> Now, of course, that doesn't come as a surprise. Some people just never > learn or listen. Or apologise. Honestly, I'd like to see proper fixes for output of "grep -r 'memcpy( ' sys/arch" , before another set of mechanical changes. --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/dec
> Maybe ahc(4) was the only one that was broken. The point is that you > could have found this by comparing the object files. Although ahc(4) has many quirks (to share sources among *BSD/Linux), only aic7xxx_osm.c belongs to ahc(4) and it doesn't have any bad cast, per output of grep adapt_dev sys/dev/ic/*. (note aic79xx_osm.c is for ahd(4)) It's always better thing to device_private()'fy, but the main trouble on device_t/softc split is that it isn't easy to check actual types (device_t or softc) against all (void *) pointers/arguments. On the other hand, (void *) casts against device_t variables are quite trivial. --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/dec
On Tue, May 12, 2009 at 05:15:05PM +0100, Robert Swindells wrote: > > Christoph Egger wrote: > >> Valeriy E. Ushakov wrote: > >> >On Tue, May 12, 2009 at 14:18:16 +, Christoph Egger wrote: > >> > >> >> struct device * -> device_t, no functional changes intended. > >> > >> >Why don't you cmp(1) the objects before and after to verify that? > >> >"Same object code generated" is, unlike intentions, something > >> >that can be verified. > >> > >> A fair number won't be the same, I would guess half our SCSI > >> drivers are currently broken. > >> > >> Do a search in sys/dev/ic for 'adapt_dev', any driver that casts > >> this to a softc instead of calling device_private() will crash. > > >The cast is a bug in drivers which have > >the device_t/softc split already done. It is harmless > >for those not yet splitted. > > Maybe ahc(4) was the only one that was broken. The point is that you > could have found this by comparing the object files. Ok, so, just to clarify: - changing struct device * to device_t is pure cosmetics - problem arise with botched device_t/softc split, which is a different thing, and *is* a functional change. That means that cegger *did* break ahc when he did the split for ahc, so thank you for fixing it. Now, of course, that doesn't come as a surprise. Some people just never learn or listen. Or apologise. -- Quentin Garnier - c...@cubidou.net - c...@netbsd.org "See the look on my face from staying too long in one place [...] every time the morning breaks I know I'm closer to falling" KT Tunstall, Saving My Face, Drastic Fantastic, 2007. pgpOYbynUv7IW.pgp Description: PGP signature
Re: CVS commit: src/sys/dev/dec
Robert Swindells wrote: > Christoph Egger wrote: >>> Valeriy E. Ushakov wrote: On Tue, May 12, 2009 at 14:18:16 +, Christoph Egger wrote: > struct device * -> device_t, no functional changes intended. Why don't you cmp(1) the objects before and after to verify that? "Same object code generated" is, unlike intentions, something that can be verified. >>> A fair number won't be the same, I would guess half our SCSI >>> drivers are currently broken. >>> >>> Do a search in sys/dev/ic for 'adapt_dev', any driver that casts >>> this to a softc instead of calling device_private() will crash. > >> The cast is a bug in drivers which have >> the device_t/softc split already done. It is harmless >> for those not yet splitted. > > Maybe ahc(4) was the only one that was broken. The point is that you > could have found this by comparing the object files. As Izumi said, struct device * and device_t are equal. Replacing a cast with device_private() generates different object code, because device_private() is a function call. Christoph
Re: CVS commit: src/sys/dev/dec
Christoph Egger wrote: >> Valeriy E. Ushakov wrote: >> >On Tue, May 12, 2009 at 14:18:16 +, Christoph Egger wrote: >> >> >> struct device * -> device_t, no functional changes intended. >> >> >Why don't you cmp(1) the objects before and after to verify that? >> >"Same object code generated" is, unlike intentions, something >> >that can be verified. >> >> A fair number won't be the same, I would guess half our SCSI >> drivers are currently broken. >> >> Do a search in sys/dev/ic for 'adapt_dev', any driver that casts >> this to a softc instead of calling device_private() will crash. >The cast is a bug in drivers which have >the device_t/softc split already done. It is harmless >for those not yet splitted. Maybe ahc(4) was the only one that was broken. The point is that you could have found this by comparing the object files. Robert Swindells
Re: CVS commit: src/sys/dev/dec
> Do a search in sys/dev/ic for 'adapt_dev', any driver that casts this > to a softc instead of calling device_private() will crash. struct device * and device_t are identical, so they won't crash unless softc will actuall be split from device_t. But I don't see benefits by mechanical changes against device_t (and device_xname()) before actual softc/device_t split. IMO, it's better to update them per drivers, to catch quirks. --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/dec
> > Valeriy E. Ushakov wrote: > >On Tue, May 12, 2009 at 14:18:16 +, Christoph Egger wrote: > > >> struct device * -> device_t, no functional changes intended. > > >Why don't you cmp(1) the objects before and after to verify that? > >"Same object code generated" is, unlike intentions, something > >that can be verified. > > A fair number won't be the same, I would guess half our SCSI > drivers are currently broken. > > Do a search in sys/dev/ic for 'adapt_dev', any driver that casts > this to a softc instead of calling device_private() will crash. The cast is a bug in drivers which have the device_t/softc split already done. It is harmless for those not yet splitted. Christoph
Re: CVS commit: src/sys/dev/dec
On Tue, May 12, 2009 at 04:24:07PM +0100, Robert Swindells wrote: > > Valeriy E. Ushakov wrote: > >On Tue, May 12, 2009 at 14:18:16 +, Christoph Egger wrote: > > >> struct device * -> device_t, no functional changes intended. > > >Why don't you cmp(1) the objects before and after to verify that? > >"Same object code generated" is, unlike intentions, something that can > >be verified. > > A fair number won't be the same, I would guess half our SCSI drivers > are currently broken. > > Do a search in sys/dev/ic for 'adapt_dev', any driver that casts this > to a softc instead of calling device_private() will crash. As much as I dislike the way cegger does those changes, that's simply not true. -- Quentin Garnier - c...@cubidou.net - c...@netbsd.org "See the look on my face from staying too long in one place [...] every time the morning breaks I know I'm closer to falling" KT Tunstall, Saving My Face, Drastic Fantastic, 2007. pgprgCXgNJjkc.pgp Description: PGP signature
Re: CVS commit: src/sys/dev/dec
Valeriy E. Ushakov wrote: >On Tue, May 12, 2009 at 14:18:16 +, Christoph Egger wrote: >> struct device * -> device_t, no functional changes intended. >Why don't you cmp(1) the objects before and after to verify that? >"Same object code generated" is, unlike intentions, something that can >be verified. A fair number won't be the same, I would guess half our SCSI drivers are currently broken. Do a search in sys/dev/ic for 'adapt_dev', any driver that casts this to a softc instead of calling device_private() will crash. Robert Swindells
Re: CVS commit: src/sys/dev/dec
On Tue, May 12, 2009 at 14:18:16 +, Christoph Egger wrote: > struct device * -> device_t, no functional changes intended. Why don't you cmp(1) the objects before and after to verify that? "Same object code generated" is, unlike intentions, something that can be verified. SY, Uwe -- u...@stderr.spb.ru | Zu Grunde kommen http://snark.ptc.spbu.ru/~uwe/ | Ist zu Grunde gehen
