From: Arnaud Lacombe <lacombar@xxxxxxxxx> Date: Wed, 23 Nov 2011 13:05:53 -0500 > > Hi, > > 2011/11/23 Reinhard Tartler <Reinhard.Tartler@xxxxxxxxxxxxxxxxxxxxxxxxxx>: > > On Mi, Nov 23, 2011 at 06:53:51 (CET), Jean Sacren wrote: > > > >> From: Michal Marek <mmarek@xxxxxxx> > >> Date: Sun, 20 Nov 2011 14:24:35 +0100 > >>> > >>> On 7.10.2011 05:29, Arnaud Lacombe wrote: > >>> > Hi, > >>> > > >>> > 2011/10/6 Reinhard Tartler <Reinhard.Tartler@xxxxxxxxxxxxxxxxxxxxxxxxxx>: > >>> >> fwrite indicates '1' written member if a zero-length string is written. > >>> > you forgot the "Signed-off-by: " part :) > >>> > >>> Reinhard, can I assume > >>> > >>> Signed-off-by: Reinhard Tartler > >>> <Reinhard.Tartler@xxxxxxxxxxxxxxxxxxxxxxxxxx> > >>> > >>> ? The patch is otherwise correct. > >> > >> I have two reasons to oppose this patch. > >> > >> 1. If 'len' value is zero, there is an issue already and it should be > >> taken care of _before_ calling fwrite(). > > > > So you're saying the function assumes a non-empty string? Why? > AFAIK, fwrite(3) is currently used: > 1) in comment printers. Empty comment are not allowed. > 2) in a callback passed to expr_print(), where the string printed is > non-empty[0] > 2) in the lexer, auto-generated, and unused > > So Jean's point is valid, but for this comment to be pedantic, I would > not weakly test for `len > 0', but enforce assumptions above with an > assertion, by both converting the existing direct call to xfwrite() > and add `assert(len > 0)' before calling fwrite(3). I prepared a patch to take care of this corner case. Review is appreciated. Thanks. -- Jean Sacren -->8 [PATCH] kbuild: Fix compiler warning with assertion when calling 'fwrite' Reinhard Tartler discovered a corner case of calling xfwrite() where the length of the string is zero. Arnaud Lacombe suggested to use assertion for the corner case, as fwrite(3) is currently used: 1) in comment printers. Empty comment are not allowed. 2) in a callback passed to expr_print(), where the string printed is either NULL OR non-empty. 3) in the lexer, auto-generated, and unused. I feel using assertion is a good solution: 1) It cleanly takes care of the above-mentioned corner case. 2) It can be easily disabled by defining NDEBUG. 3) It asserts xfwrite() is simply a wrapper for fwrite(). Reported-by: Reinhard Tartler <Reinhard.Tartler@xxxxxxxxxxxxxxxxxxxxxxxxxx> Signed-off-by: Arnaud Lacombe <lacombar@xxxxxxxxx> Signed-off-by: Jean Sacren <sakiwit@xxxxxxxxx> --- scripts/kconfig/expr.h | 1 + scripts/kconfig/lkc.h | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h index 80fce57..d4ecce8 100644 --- a/scripts/kconfig/expr.h +++ b/scripts/kconfig/expr.h @@ -10,6 +10,7 @@ extern "C" { #endif +#include <assert.h> #include <stdio.h> #ifndef __cplusplus #include <stdbool.h> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h index b633bdb..c18f2bd 100644 --- a/scripts/kconfig/lkc.h +++ b/scripts/kconfig/lkc.h @@ -90,8 +90,10 @@ struct conf_printer { /* confdata.c and expr.c */ static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out) { - if (fwrite(str, len, count, out) < count) - fprintf(stderr, "\nError in writing or end of file.\n"); + assert(len != 0); + + if (fwrite(str, len, count, out) != count) + fprintf(stderr, "Error in writing or end of file.\n"); } /* menu.c */ -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html