Re: why cast to char* through void*

["Greg A. Woods" <woods%planix.com@localhost>]

At Thu, 2 Jul 2009 14:59:39 -0400, christos%zoulas.com@localhost (Christos 
Zoulas) wrote:
Subject: Re: why cast to char* through void*
> 
> On Jul 2,  2:34pm, woods%planix.com@localhost ("Greg A. Woods") wrote:
> -- Subject: Re: why cast to char* through void*
> 
> | No, I didn't try lint.  But lint didn't "break" -- lint warnings are
> | normal in many cases and should NOT be hidden or obscured by even more
> | questionable practises.
> 
> I agree, and lint should be fixed instead of polluting the code with linted
> comments.

I don't think lint can, or needs to be, fixed in this case though.

I.e. the lint warning is valid in most cases -- this isn't necessarily a
safe construct.

Only in specific cases can the programmer decide that this kind of
language usage is actually going to work out in exactly the way it
should.  That's what the "LINTED" comment means -- it's intended for
exactly these cases where we can say we've checked the code carefully
and we now know it is correct despite the fact it uses an idiom that is
questionable in all cases where we have not done the same careful checks
after having been warned the first time about the usage.

One could, I suppose, argue that the code could also be changed to be
less likely to trigger such warnings in the first place.

Perhaps one could use separate storage areas for the array of pointers
and the setenv string list, thus avoiding the type punning necessary to
change the type of the storage object right in its middle, but perhaps
that wouldn't be as "cool" a thing to do....  2 malloc()s vs. one, etc.

here's another diff, with arguably better internal docs:

--- login_cap.c 10 Feb 2007 12:57:39 -0500      1.25
+++ login_cap.c 02 Jul 2009 19:45:10 -0400      
@@ -510,23 +510,37 @@
                        ptr++;
        }
 
-       /* allocate ptr array and string */
+       /* allocate storage for the ptr array and the string it points to */
        count = i;
        res = malloc(count * sizeof(char *) + strlen(str) + 1);
 
        if (!res)
                return -1;
        
-       ptr = (char *)(void *)&res[count];
-       (void)strcpy(ptr, str);
+       /*
+        * find the first byte after the end of the pointer array so that we
+        * can copy the string into place there
+        */
+       /* LINTED (we are storing the string of variable names and values in
+        * the same storage as the array of pointers which point into that
+        * string at each variable name) */
+       ptr = (char *) &res[count];
+       (void) strcpy(ptr, str);
 
-       /* split string */
+       /*
+        * split the string into "envvar[=value]" strings, assigning a pointer
+        * in the array at the beginning of res to each consecutive one
+        */
        for (i = 0; (res[i] = stresep(&ptr, stop, '\\')) != NULL; )
                if (*res[i])
                        i++;
        
        count = i;
 
+       /*
+        * call (*senv)() [normally setenv() via envset() above] to set each
+        * given variable in the caller's environment
+        */
        for (i = 0; i < count; i++) {
                if ((ptr = strchr(res[i], '=')) != NULL)
                        *ptr++ = '\0';


-- 
                                                Greg A. Woods
                                                Planix, Inc.

<woods%planix.com@localhost>       +1 416 218-0099        http://www.planix.com/

Attachment: pgp0EQMBA0_uT.pgp
Description: PGP signature