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. --apb (Alan Barrett)
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. martin discovered that this one works: const char fmt[] = ...; .mrg.
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: 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
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
