[imapfilter-devel] Compiler Warnings

[David DeSimone]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael Skibbe <mskibbe at suse.de> wrote:
>
> i see that rsnapshot has some little compiler warnings.
> i fixed these problems and create a patch. What do you say about this?

These changes seem a bit dubious.  What are you trying to fix?

> --- cert.c
> +++ cert.c
> @@ -163,11 +163,13 @@
>  	FILE *fd;
>  	char b, c, buf[64];
>  	char *certf;
> +	char *rt;
>  
>  	do {
>  		printf("(R)eject, accept (t)emporarily or "
>  		    "accept (p)ermanently? ");
> -		fgets(buf, sizeof(buf), stdin);
> +		rt = fgets(buf, sizeof(buf), stdin);
> +		free (rt);
>  		c = tolower((int)(*buf));
>  	} while (c != 'r' && c != 't' && c != 'p');

In the normal case, fgets() returns "buf".  It does not return a
malloc'd location, so there is no reason to free() it.  In fact, in the
above case you are freeing a stack buffer, which looks wrong to me.

>  
> @@ -210,11 +212,13 @@
>  mismatch_cert(void)
>  {
>  	char c, buf[64];
> +	char *rt;
>  
>  	do {
>  		printf("ATTENTION: SSL/TLS certificate fingerprint mismatch.\n"
>  		    "Proceed with the connection (y/n)? ");
> -		fgets(buf, sizeof(buf), stdin);
> +		rt = fgets(buf, sizeof(buf), stdin);
> +		free(rt);
>  		c = tolower((int)(*buf));
>  	} while (c != 'y' && c != 'n');

Same again here.

> --- system.c
> +++ system.c
> @@ -211,8 +211,10 @@
>  	luaL_checktype(lua, 1, LUA_TUSERDATA);
>  	luaL_checktype(lua, 2, LUA_TSTRING);
>  
> -	fwrite(lua_tostring(lua, 2), sizeof(char), strlen(lua_tostring(lua,
> +	size_t rt;
> +	rt = fwrite(lua_tostring(lua, 2), sizeof(char), strlen(lua_tostring(lua,
>  	    2)), *(FILE **) (lua_touserdata(lua, 1)));
> +	free(&rt);
>  
>  	lua_pop(lua, 2);

In this case you are declaring a variable in the middle of a block of
code, which doesn't seem like something all compilers would support.

> @@ -283,8 +285,10 @@
>  	close(STDOUT_FILENO);
>  	close(STDERR_FILENO);
>  	if (open("/dev/null", O_RDWR) != -1) {
> -		dup(STDIN_FILENO);
> -		dup(STDIN_FILENO);
> +		int rt = 0;
> +		rt = dup(STDIN_FILENO);
> +		rt = dup(STDIN_FILENO);
> +		free(&rt);
>  	}
>  
>  	return 0;

In this case, dup() returns an integer, not a pointer, and freeing the
pointer to that integer will not do any good.


In all of the above cases, you should be able to avoid your compiler
warnings, not by capturing and freeing the return value, but instead by
casting the return value to (void).  In all cases above, the intent is
to use the side-effect of the function, so ignoring the return value is
reasonable and valid.  Although of course, better error checking would
be nice...

- -- 
David DeSimone == Network Admin == fox at verio.net
  "It took me fifteen years to discover that I had no
   talent for writing, but I couldn't give it up because
   by that time I was too famous.  -- Robert Benchley
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFE0k91FSrKRjX5eCoRAvh2AJ9q7YZ8LIH2gFnDEQSgv6NPM1P55QCfexct
/n33qTgJ8KowIOoB7NUlDgA=
=uyQ/
-----END PGP SIGNATURE-----