Re: CVS commit: src/sys/fs/tmpfs
On Thu, Aug 18, 2011 at 09:51:50PM +, Taylor R Campbell wrote: > Forgot to add that this also fixes a space leak in tmpfs_rename, > introduced a couple months ago, which nobody reported as far as I > know. The leak sometimes caused tmpfs_renamerace_dirs to fail with > ENOSPC. The problem was that renaming a directory over an empty > directory would fail to decrement the target's link count enough, so > that the target would never be released. There was a thread somewhere a while back (in tech-kern I think) about refcount leaks in tmpfs. As I recall what changed a couple months ago was a semi-compensating error somewhere else... or maybe that was earlier. -- David A. Holland dholl...@netbsd.org
Re: src/sys/modules/spdmem
On Thu, Aug 18, 2011 at 01:51:33PM -0500, David Young wrote: > > Rather than sweeping the issue under the rug, wouldn't it be better to > > actually fix the problem? > > > > See attached diff which replaces the "variable" format with a > > literal #define string ... > > I think we should make no changes to appease the compiler in this case. > There is nothing inherently safer about using a literal format string > than a static const format string, the compiler just isn't smart enough > to tell an unsafe non-literal format string from a safe one. That's not entirely true; e.g. if the compiler can't figure out that the format string is constant, it won't catch stuff like const char format[] = "%d"; : printf(format, "wrong"); which it otherwise would. I would lean towards fixing the ones that can be fixed noninvasively; particularly in old code the motivation for the status quo seems to have been manually saving a few bytes on string constants... which the toolchain should do automatically these days. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/fs/tmpfs
Forgot to add that this also fixes a space leak in tmpfs_rename, introduced a couple months ago, which nobody reported as far as I know. The leak sometimes caused tmpfs_renamerace_dirs to fail with ENOSPC. The problem was that renaming a directory over an empty directory would fail to decrement the target's link count enough, so that the target would never be released.
Re: src/sys/modules/spdmem
On Thu, Aug 18, 2011 at 11:11:20AM -0700, Paul Goyette wrote: > >Module Name:src > >Committed By: christos > >Date: Thu Aug 18 17:02:49 UTC 2011 > > > >Modified Files: > >src/sys/modules/spdmem: Makefile > > > >Log Message: > >document non-literal string format > > Rather than sweeping the issue under the rug, wouldn't it be better to > actually fix the problem? > > See attached diff which replaces the "variable" format with a > literal #define string ... I think we should make no changes to appease the compiler in this case. There is nothing inherently safer about using a literal format string than a static const format string, the compiler just isn't smart enough to tell an unsafe non-literal format string from a safe one. Dave -- David Young OJC Technologies dyo...@ojctech.com Urbana, IL * (217) 344-0444 x24
RE: src/sys/modules/spdmem
Module Name:src Committed By: christos Date: Thu Aug 18 17:02:49 UTC 2011 Modified Files: src/sys/modules/spdmem: Makefile Log Message: document non-literal string format Rather than sweeping the issue under the rug, wouldn't it be better to actually fix the problem? See attached diff which replaces the "variable" format with a literal #define string ... - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -Index: spdmem.c === RCS file: /cvsroot/src/sys/dev/ic/spdmem.c,v retrieving revision 1.3 diff -u -p -r1.3 spdmem.c --- spdmem.c1 Aug 2011 03:49:52 - 1.3 +++ spdmem.c18 Aug 2011 18:06:39 - @@ -124,7 +124,7 @@ static const uint16_t spdmem_cycle_frac[ }; /* Format string for timing info */ -static const char* latency="tAA-tRCD-tRP-tRAS: %d-%d-%d-%d\n"; +#defineLATENCY "tAA-tRCD-tRP-tRAS: %d-%d-%d-%d\n"; /* sysctl stuff */ static int hw_node = CTL_EOL; @@ -554,7 +554,7 @@ decode_sdram(const struct sysctlnode *no if (s->sm_sdr.sdr_tCAS & (1 << i)) tAA = i; tAA++; - aprint_verbose_dev(self, latency, tAA, s->sm_sdr.sdr_tRCD, + aprint_verbose_dev(self, LATENCY, tAA, s->sm_sdr.sdr_tRCD, s->sm_sdr.sdr_tRP, s->sm_sdr.sdr_tRAS); decode_voltage_refresh(self, s); @@ -596,7 +596,7 @@ decode_ddr(const struct sysctlnode *node #define __DDR_ROUND(scale, field) \ ((scale * s->sm_ddr.field + cycle_time - 1) / cycle_time) - aprint_verbose_dev(self, latency, tAA, __DDR_ROUND(250, ddr_tRCD), + aprint_verbose_dev(self, LATENCY, tAA, __DDR_ROUND(250, ddr_tRCD), __DDR_ROUND(250, ddr_tRP), __DDR_ROUND(1000, ddr_tRAS)); #undef __DDR_ROUND @@ -640,7 +640,7 @@ decode_ddr2(const struct sysctlnode *nod #define __DDR2_ROUND(scale, field) \ ((scale * s->sm_ddr2.field + cycle_time - 1) / cycle_time) - aprint_verbose_dev(self, latency, tAA, __DDR2_ROUND(250, ddr2_tRCD), + aprint_verbose_dev(self, LATENCY, tAA, __DDR2_ROUND(250, ddr2_tRCD), __DDR2_ROUND(250, ddr2_tRP), __DDR2_ROUND(1000, ddr2_tRAS)); #undef __DDR_ROUND @@ -691,7 +691,7 @@ decode_ddr3(const struct sysctlnode *nod #define__DDR3_CYCLES(field) (s->sm_ddr3.field / s->sm_ddr3.ddr3_tCKmin) - aprint_verbose_dev(self, latency, __DDR3_CYCLES(ddr3_tAAmin), + aprint_verbose_dev(self, LATENCY, __DDR3_CYCLES(ddr3_tAAmin), __DDR3_CYCLES(ddr3_tRCDmin), __DDR3_CYCLES(ddr3_tRPmin), (s->sm_ddr3.ddr3_tRAS_msb * 256 + s->sm_ddr3.ddr3_tRAS_lsb) / s->sm_ddr3.ddr3_tCKmin); @@ -725,7 +725,7 @@ decode_fbdimm(const struct sysctlnode *n #define__FBDIMM_CYCLES(field) (s->sm_fbd.field / s->sm_fbd.fbdimm_tCKmin) - aprint_verbose_dev(self, latency, __FBDIMM_CYCLES(fbdimm_tAAmin), + aprint_verbose_dev(self, LATENCY, __FBDIMM_CYCLES(fbdimm_tAAmin), __FBDIMM_CYCLES(fbdimm_tRCDmin), __FBDIMM_CYCLES(fbdimm_tRPmin), (s->sm_fbd.fbdimm_tRAS_msb * 256 + s->sm_fbd.fbdimm_tRAS_lsb) /
Re: CVS commit: src/sys/arch/ofppc/ofppc
On Thu, Aug 18, 2011 at 08:55:44AM +, Frank Wille wrote: > Module Name: src > Committed By: phx > Date: Thu Aug 18 08:55:43 UTC 2011 > > Modified Files: > src/sys/arch/ofppc/ofppc: disksubr.c > > Log Message: > First check whether an MBR is present. Then use it for locating the disklabel. > Otherwise try to construct a disklabel from RDB partitions, and when > everything fails, look for a raw NetBSD disklabel in LABELSECTOR. > This is the same sequence as in ofwboot now. > Also fixed some typos in the comments. Might be worth looking/using/updating the MI(ish) code that i386 uses when playing 'hunt the label'. After all disks get moved between machines. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/external/bsd/mdocml
On Thu, Aug 18, 2011 at 01:47:19AM +, Christos Zoulas wrote: > In article <20110817212805.gb16...@britannica.bec.de>, > Joerg Sonnenberger wrote: > >Could you please stop randomly changing 3rd party code without > >contacting the maintainer? > > Unless the rules have changed, for simple compilation fixes we don't do that. > Otherwise the overhead of doing so would be too large. In this particular > case, do you have a different/better way of fixing this? If yes, I'd like > to know now. How can it be a simple compilation fix if you also change the Makefile? You are craeting additional work for every future import without providing much value. This is not specific to this case, but many other of your recent changes to stuff that can just as well be changed upstream first and reimported later. Joerg
Re: CVS commit: src/usr.bin/unzip
In article <20110818051549.gb24...@arresum.veego.de>, Bernd Ernesti wrote: >On Thu, Aug 18, 2011 at 01:49:30AM +, Christos Zoulas wrote: >> In article <20110817212950.gc16...@britannica.bec.de>, >> Joerg Sonnenberger wrote: >> >Did you test this change for breaking compatibility with 3rd party >> >scripts that parse the output of unzip? >> > >> >> I thought about that, but where does one find such scripts? Providing >> a flag to make backwards compatible output is another way of doing it, >> but is it worth it? > >No, just back the change out because the output should look the same as >the 'normal' unzip does. >The change is nice but should not be done as long as the official unzip >still use the old output. Fine, reverted. christos
re: CVS commit: src
> Also with those gcc45 libs, they are distributed amongst the .WAITs but > they don't need to be since there is no dependency as just the .a files > are produced now.. strictly, the dependency is introduced at link time, > not build time.. oh, good point. can you do it for libs? we can't do that for tools/ side at least, since ./configure won't work without the depends installed, even for libfoo.a. thanks. .mrg.
re: CVS commit: src
On Thu, 18 Aug 2011, matthew green wrote: > > > > Log Message: > > > build GMP, MPFR and MPC as private libraries just for GCC. don't > > > install the headers or librarys into the system. > > > > in lib/Makefile should this really be > > > > .if (${MKGCC} != "no") && ${HAVE_GCC} >= 45 > > > > rather than > > > > .if defined(HAVE_GCC) && ${HAVE_GCC} >= 45 > > > > ? > > > > I think that HAVE_GCC is always defined, these days.. > > it should have both. we shouldn't rely on HAVE_GCC always > being defined. i thought it wasn't defined for pcc builds? It was that way in the past, but I think the AVAILABLE_COMPILERS thing that Jörg introduced did away with that, so HAVE_etc is always defined..? Also with those gcc45 libs, they are distributed amongst the .WAITs but they don't need to be since there is no dependency as just the .a files are produced now.. strictly, the dependency is introduced at link time, not build time.. iain