Colin Watson wrote:
> Firstly, the trim module fails to add its files to lib_SOURCES, so they
> don't get built.
>
> Secondly, after fixing that, it tries to include "mbuiter.h" without
> depending on that module. On inspection, I found that it claims to be
> including it for MB_CUR_MAX, but that's actually in <stdlib.h> (the only
> system header mbuiter.h includes over and above mbiter.h) so including
> "mbuiter.h" for it seems odd and redundant. I added a check for
> defined(MB_CUR_MAX) because I don't know offhand if C89 <stdlib.h>
> guarantees that; if it does, please remove this check again.
>
> Thirdly, trim.c uses isspace unconditionally so it should also include
> <ctype.h> unconditionally.
Thanks for reporting all this. I'm applying the appended patch to fix
all this.
> Fourthly, gcc complains:
>
> trim.c: In function ‘trim2’:
> trim.c:67: warning: ‘r’ may be used uninitialized in this function
>
> ... and it may be that the state machine in fact renders it impossible
> for this to be used uninitialised but this wasn't obvious to me.
I added a comment with the invariant.
> Some more sanity-checking seems to be called for here.
I don't agree. The code is fine. Some day gcc will learn not to emit
"uninitialized" warnings for cases like this. The code that you put in
makes the code flow needlessly more convoluted.
Bruno
2007-10-18 Colin Watson <[EMAIL PROTECTED]> (tiny change)
Bruno Haible <[EMAIL PROTECTED]>
* lib/trim.c: Include config.h unconditionally. Include trim.h always.
Include ctype.h always. Include stdlib.h, not mbuiter.h, for MB_CUR_MAX.
* modules/trim (Depends-on): Add mbchar.
(configure.ac): Add gl_FUNC_MBRTOWC.
(Makefile.am): Augment lib_SOURCES.
--- lib/trim.c.orig 2007-10-18 03:47:05.000000000 +0200
+++ lib/trim.c 2007-10-18 03:28:14.000000000 +0200
@@ -1,5 +1,5 @@
/* Removes leading and/or trailing whitespaces
- Copyright (C) 2006 Free Software Foundation, Inc.
+ Copyright (C) 2006-2007 Free Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -16,21 +16,21 @@
/* Written by Davide Angelocola <[EMAIL PROTECTED]> */
-#ifdef HAVE_CONFIG_H
-# include <config.h>
-#endif
+#include <config.h>
+
+/* Specification. */
+#include "trim.h"
+
+#include <ctype.h>
#if HAVE_MBRTOWC
# include <stddef.h>
+# include <stdlib.h>
# include "mbchar.h"
# include "mbiter.h"
-# include "mbuiter.h" /* FIXME: for MB_CUR_MAX */
-#else
-# include <ctype.h>
#endif
#include "xalloc.h"
-#include "trim.h"
char *
trim2(const char *s, int how)
@@ -62,7 +62,7 @@
if (how != TRIM_LEADING)
{
int state = 0;
- char *r;
+ char *r; /* used only while state = 2 */
mbi_init (i, d, strlen (d));
--- modules/trim.orig 2007-10-18 03:47:05.000000000 +0200
+++ modules/trim 2007-10-18 03:23:06.000000000 +0200
@@ -2,16 +2,19 @@
trim() removes leading and/or trailing whitespaces
Files:
-lib/trim.c
lib/trim.h
+lib/trim.c
Depends-on:
xalloc
+mbchar
mbiter
configure.ac:
+gl_FUNC_MBRTOWC
Makefile.am:
+lib_SOURCES += trim.c
Include:
#include "trim.h"
