Re: CVS commit: src/external/cddl/osnet
On Apr 24, 3:23am, rm...@netbsd.org (Mindaugas Rasiukevicius) wrote: -- Subject: Re: CVS commit: src/external/cddl/osnet | chris...@zoulas.com (Christos Zoulas) wrote: | > | > | Are you planning to maintain this? That should be something for the | > | maintainer to decide, since it is not a small and simple code base. | > | > What I am saying is that all lines you change, they become diffs. Diffs | > in our preferred style or theirs does not matter. Either way we maintain | > them. I don't even think that people at sun^Woracle will think twice about | > taking our changes as they are (without changing the style of sizeof). | | ZFS/DTrace is, at some level, abstracted by the wrappers to our NetBSD | interfaces. There are certain heavy changes, which in any case require | maintenance. However, there is a difference between maintaining certain | set of functions and core changes, and a ton of style, whitespace, etc | changes all over the place. Do not you agree? I am saying if you are going to change the line, change it the way it is supposed to. I am not advocating reformat/change the whole tree for no good reason. | > Finally I don't think we'll see much of the Open Source stuff sun freed | > keep being updated by Oracle. Haven't you been following the news lately? | | That seems like an argument based on a rumour. :) Where is the March OpenSolaris release? What happened to the pay for Solaris model? Why does the opendoc office converter now costs $90? | OK. Thanks. No problem. christos
Re: CVS commit: src/external/cddl/osnet
chris...@zoulas.com (Christos Zoulas) wrote: > > | Are you planning to maintain this? That should be something for the > | maintainer to decide, since it is not a small and simple code base. > > What I am saying is that all lines you change, they become diffs. Diffs > in our preferred style or theirs does not matter. Either way we maintain > them. I don't even think that people at sun^Woracle will think twice about > taking our changes as they are (without changing the style of sizeof). ZFS/DTrace is, at some level, abstracted by the wrappers to our NetBSD interfaces. There are certain heavy changes, which in any case require maintenance. However, there is a difference between maintaining certain set of functions and core changes, and a ton of style, whitespace, etc changes all over the place. Do not you agree? > Finally I don't think we'll see much of the Open Source stuff sun freed > keep being updated by Oracle. Haven't you been following the news lately? That seems like an argument based on a rumour. :) > | That is arbitrary. Can we please just stay relatively liberal (up to sane > | point) about small style differences each developer has? Otherwise we can > | bikeshed about this until November. > > I don't think that this is controvercial or a big deal. It is not a mandate > to go and change all existing code, or force people to do it. OK. Thanks. -- Mindaugas
Re: CVS commit: src/external/cddl/osnet
David Holland wrote: > >> > >> That was not an explicit preference, it was just a random choice, so I > >> have corrected it. > > > > That is arbitrary. Can we please just stay relatively liberal (up to > > sane point) about small style differences each developer has? Otherwise > > we can bikeshed about this until November. > > Take it up with the comp.lang.c FAQ :-) Thanks, but I will leave that to you. ;) > > We should make lint examine the sizes of malloc calls... > Lint would not be able to raise above its current level. Coverity might help in this case, as it is more clever and tries to do some better tracking amongst the layers. Possibly it can be taught to do better examination as well. -- Mindaugas
Re: CVS commit: src/external/cddl/osnet
On Apr 24, 1:43am, dholland-sourcechan...@netbsd.org (David Holland) wrote: -- Subject: Re: CVS commit: src/external/cddl/osnet | We should make lint examine the sizes of malloc calls... We have coverity for that but unfortunately the free runs have been busted for a long time. christos
Re: CVS commit: src/external/cddl/osnet
On Apr 24, 2:09am, rm...@netbsd.org (Mindaugas Rasiukevicius) wrote: -- Subject: Re: CVS commit: src/external/cddl/osnet | Are you planning to maintain this? That should be something for the | maintainer to decide, since it is not a small and simple code base. What I am saying is that all lines you change, they become diffs. Diffs in our preferred style or theirs does not matter. Either way we maintain them. I don't even think that people at sun^Woracle will think twice about taking our changes as they are (without changing the style of sizeof). Finally I don't think we'll see much of the Open Source stuff sun freed keep being updated by Oracle. Haven't you been following the news lately? | > | Also, while I see the benefits with *var (technically, it is against KNF, | > | by the way), my preference is to use type. | > | > That was not an explicit preference, it was just a random choice, so I have | > corrected it. | | That is arbitrary. Can we please just stay relatively liberal (up to sane | point) about small style differences each developer has? Otherwise we can | bikeshed about this until November. I don't think that this is controvercial or a big deal. It is not a mandate to go and change all existing code, or force people to do it. christos
Re: CVS commit: src/external/cddl/osnet
On Sat, Apr 24, 2010 at 02:09:57AM +0100, Mindaugas Rasiukevicius wrote: >> | Also, while I see the benefits with *var (technically, it is against KNF, >> | by the way), my preference is to use type. >> >> That was not an explicit preference, it was just a random choice, so I have >> corrected it. > > That is arbitrary. Can we please just stay relatively liberal (up to sane > point) about small style differences each developer has? Otherwise we can > bikeshed about this until November. Take it up with the comp.lang.c FAQ :-) (I more or less agree with you, but they've put a lot more bikeshed time into that than we can manage even between now and November, and they're not wrong.) We should make lint examine the sizes of malloc calls... -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/external/cddl/osnet
chris...@zoulas.com (Christos Zoulas) wrote: > | > | It is consistent with the code in OpenSolaris, so it should not diverge. > > This is changed code that would produce a diff in either case. > It is more correct, and I don't see propagating a poor choice. > OpenSolaris uses NULL for 0 in many places, should we do the same > and turn off compiler warnings? Are you planning to maintain this? That should be something for the maintainer to decide, since it is not a small and simple code base. > | Also, while I see the benefits with *var (technically, it is against KNF, > | by the way), my preference is to use type. > > That was not an explicit preference, it was just a random choice, so I have > corrected it. That is arbitrary. Can we please just stay relatively liberal (up to sane point) about small style differences each developer has? Otherwise we can bikeshed about this until November. -- Mindaugas
Re: CVS commit: src/external/cddl/osnet
On Apr 24, 1:43am, rm...@netbsd.org (Mindaugas Rasiukevicius) wrote: -- Subject: Re: CVS commit: src/external/cddl/osnet | chris...@astron.com (Christos Zoulas) wrote: | > >Modified Files: | > > src/external/cddl/osnet/dev/dtrace: dtrace_sysctl.c dtrace_unload.c | > > src/external/cddl/osnet/dev/dtrace/amd64: dtrace_subr.c | > > src/external/cddl/osnet/dev/dtrace/i386: dtrace_subr.c | > > src/external/cddl/osnet/dist/uts/common/dtrace: dtrace.c | > > | > >Log Message: | > >Remove a couple of zero length kmem_frees. | > > | > >It should fix at least one crash when unloading the dtrace module, | > >possibly many others. | > | > please use sizeof(*var) instead of sizeof(type) | > | | It is consistent with the code in OpenSolaris, so it should not diverge. This is changed code that would produce a diff in either case. It is more correct, and I don't see propagating a poor choice. OpenSolaris uses NULL for 0 in many places, should we do the same and turn off compiler warnings? | Also, while I see the benefits with *var (technically, it is against KNF, | by the way), my preference is to use type. That was not an explicit preference, it was just a random choice, so I have corrected it. christos
Re: CVS commit: src/external/cddl/osnet
chris...@astron.com (Christos Zoulas) wrote: > >Modified Files: > > src/external/cddl/osnet/dev/dtrace: dtrace_sysctl.c dtrace_unload.c > > src/external/cddl/osnet/dev/dtrace/amd64: dtrace_subr.c > > src/external/cddl/osnet/dev/dtrace/i386: dtrace_subr.c > > src/external/cddl/osnet/dist/uts/common/dtrace: dtrace.c > > > >Log Message: > >Remove a couple of zero length kmem_frees. > > > >It should fix at least one crash when unloading the dtrace module, > >possibly many others. > > please use sizeof(*var) instead of sizeof(type) > It is consistent with the code in OpenSolaris, so it should not diverge. Also, while I see the benefits with *var (technically, it is against KNF, by the way), my preference is to use type. -- Mindaugas
Re: CVS commit: src/external/cddl/osnet
In article <20100423113953.7c16417...@cvs.netbsd.org>, Adam Hoka wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: ahoka >Date: Fri Apr 23 11:39:53 UTC 2010 > >Modified Files: > src/external/cddl/osnet/dev/dtrace: dtrace_sysctl.c dtrace_unload.c > src/external/cddl/osnet/dev/dtrace/amd64: dtrace_subr.c > src/external/cddl/osnet/dev/dtrace/i386: dtrace_subr.c > src/external/cddl/osnet/dist/uts/common/dtrace: dtrace.c > >Log Message: >Remove a couple of zero length kmem_frees. > >It should fix at least one crash when unloading the dtrace module, >possibly many others. please use sizeof(*var) instead of sizeof(type) christos
Re: CVS commit: src/lib/libc/compat/gen
In article <20100423190454.62ec017...@cvs.netbsd.org>, Matthias Drochner wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: drochner >Date: Fri Apr 23 19:04:54 UTC 2010 > >Modified Files: > src/lib/libc/compat/gen: Makefile.inc compat_frexp_ieee754.c > compat_ldexp_ieee754.c compat_modf_ieee754.c > >Log Message: >use the local versions of ldexp/frexp/modf again rather than pulling >in libm source code. The libm functions depend on other libm functions, >this requires symbol renaming, and with the reachover method this >is going to be a mess. Also, bundling the dependencies into one .o >file has the potential to cause symbol conflicts on static linking. > You should comment on the PR that started all this in the first place (different results). christos
Re: CVS commit: src/distrib
On Fri, 23 Apr 2010, Havard Eidnes wrote:
> Modified Files:
> src/distrib/i386/cdroms: Makefile.cdrom
> src/distrib/sparc64/cdroms/installcd: Makefile
>
> Log Message:
> Um, as has been noted, INSTALL_FILE records what's installed if it's
> done unprived, and that doesn't go down well on a re-build. So use
> ${INSTALL} ${COPY} instead to fix this problem.
I think that this should use ${HOST_INSTALL_FILE} -m ${SOME_MODE}.
Similarly, I think the ${CP}+${CHMOD} pairs should use HOST_INSTALL_FILE,
and the ${MKDIR} commands should use HOST_INSTALL_DIR.
--apb (Alan Barrett)
Re: CVS commit: src/distrib
> On Thu, Apr 22, 2010 at 09:37:51PM +, Tom Spindler wrote:
> > > Log Message:
> > > Change use of ${CP} and ${CHMOD} to ${INSTALL_FILE}, so that if the
> > > target is made un-writeable, the build won't bomb out during an
> > > UPDATE build. OK'ed by mar...@.
> >
> > on i386, when I try building, I now get:
> > /usr/local/locdisk/locobj/tools/bin/nbmtree -CSM -k all -N /src/nbsrc/etc
> > > >/usr/local/locdisk/locobj/i386/src/nbsrc/destdir.i386/METALOG.new
> > nbmtree: ./cdrom: missing directory in specification
> > nbmtree: failed at line 34934 of the specification
> > ***
> > [/usr/local/locdisk/locobj/i386/src/nbsrc/destdir.i386/METALOG.sanitised]
> > Error code 1
> > 1 error
> >
> > nbmake: stopped in /src/nbsrc/distrib/sets
> >
> > For some reason, sparc64 does build successfully, however.
>
> I wonder if it should be HOST_INSTALL_FILE instead of INSTALL_FILE here?
Yes, I saw that. Sorry for not adequatly testing...
Either that, or ${INSTALL} ${COPY}.
Regards,
- Håvard
Re: CVS commit: src/distrib
On Thu, Apr 22, 2010 at 09:37:51PM +, Tom Spindler wrote:
> > Log Message:
> > Change use of ${CP} and ${CHMOD} to ${INSTALL_FILE}, so that if the
> > target is made un-writeable, the build won't bomb out during an
> > UPDATE build. OK'ed by mar...@.
>
> on i386, when I try building, I now get:
> /usr/local/locdisk/locobj/tools/bin/nbmtree -CSM -k all -N /src/nbsrc/etc
> >/usr/local/locdisk/locobj/i386/src/nbsrc/destdir.i386/METALOG.new
> nbmtree: ./cdrom: missing directory in specification
> nbmtree: failed at line 34934 of the specification
> *** [/usr/local/locdisk/locobj/i386/src/nbsrc/destdir.i386/METALOG.sanitised]
> Error code 1
> 1 error
>
> nbmake: stopped in /src/nbsrc/distrib/sets
>
> For some reason, sparc64 does build successfully, however.
>
I wonder if it should be HOST_INSTALL_FILE instead of INSTALL_FILE here?
Martin
