Re: src/sys/modules/spdmem
On Fri, Aug 19, 2011 at 07:49:55PM +1000, matthew green wrote: > > If it helped, then it would also be fine to change > > const char * format = "%s"; > > to > > const char * const format = "%s"; > > but it doesn't help. > > martin discovered that this one works: > >const char fmt[] = "..."; Which is the better choice anyway as it saves 4 or 8 bytes of static data. Kind regards -- Matthias Scheler http://zhadum.org.uk/
re: src/sys/modules/spdmem
> On Fri, 19 Aug 2011, David Holland wrote: > >> I think we should make no changes to appease the compiler in > >> this case. [...] > > > > I would lean towards fixing the ones that can be fixed > > noninvasively; [...] > > The compiler is being really stupid, and I don't like making > invasive changes to appease it. Non-invasive changes are fine, > such as adding CFLAGS.filename += -Wno-stupid-warning to a > Makefile, or changing a const variable to a #define. > > If it helped, then it would also be fine to change > const char * format = "%s"; > to > const char * const format = "%s"; > but it doesn't help. martin discovered that this one works: const char fmt[] = "..."; .mrg.
Re: src/sys/modules/spdmem
On Fri, Aug 19, 2011 at 08:07:11AM +0200, Alan Barrett wrote: > If it helped, then it would also be fine to change > const char * format = "%s"; > to > const char * const format = "%s"; > but it doesn't help. but: const char format[] ="%s"; works just fine. Martin
Re: src/sys/modules/spdmem
On Fri, 19 Aug 2011, David Holland wrote: I think we should make no changes to appease the compiler in this case. [...] I would lean towards fixing the ones that can be fixed noninvasively; [...] The compiler is being really stupid, and I don't like making invasive changes to appease it. Non-invasive changes are fine, such as adding CFLAGS.filename += -Wno-stupid-warning to a Makefile, or changing a const variable to a #define. If it helped, then it would also be fine to change const char * format = "%s"; to const char * const format = "%s"; but it doesn't help. --apb (Alan Barrett)
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: 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) /
