David Mathog wrote:[color=blue]
> This one is driving me slightly batty. The code in question is
> buried deep in somebody else's massive package but it boils down to
> this, two pointers are declared, the first is:
>
> char **resname
>
> which is part of the "atoms" structure that is passed into the function
> from the outside. I have not yet found where it is allocated but I'm
> reasonably sure from other chunks of this code that it was by:
>
> atoms->resname=malloc(sizeof(char**)*SOMENUMBER);[/color]
This is just the first of several type errors -- or maybe
not, because you say this particular allocation is a matter of
hypothesis and not observation. Observe that sizeof(char**)
ought to be sizeof(char*), and the two might be different on
some machines -- word-addressed machines, for example. This
kind of confusion is one reason why the formulation
atoms->resname = malloc(SOMENUMBER * sizeof *atoms->resname)
is generally favored on c.l.c.
But this may not be a problem, since you don't actually know
that the storage is allocated in this erroneous manner.
[color=blue]
> Within my function these are found:
>
> char **pnew_res_names;
>
> pnew_res_names=malloc(sizeof(char*)*10);[/color]
This one is right, although again the c.l.c.-favored formula
would be less error-prone. Note that the newly-allocated storage
has indeterminate content; it is not initialized to anything in
particular. (This will become important later on.)
[color=blue]
> /* char ***, pointers to pointers to pointers to the name strings */
> (void) fprintf(stderr,"0e: %8x\n",(void *)atoms->resname);[/color]
Here and below, some variant of "%p" would be preferable to
"%x". The latter handles integers, which a `void*' is not. Also,
the comment is wrong: `atoms->resname' is a `char**', not a `char***'
(or so you said at the beginning).
[color=blue]
> /* char **, pointers to pointers to the name strings */
> (void) fprintf(stderr,"0f: %8x\n",(void *)(atoms->resname[0]));
> (void) fprintf(stderr,"1f: %8x\n",(void *)(atoms->resname[1]));
> (void) fprintf(stderr,"2f: %8x\n",(void *)(atoms->resname[2]));
> /* char *, pointers to the first characters in the name strings */
> (void) fprintf(stderr,"0g: %8x\n",(void *) *(atoms->resname[0]));[/color]
These three are bogus. You've started with a `char**' and
done two levels of indirection, so what you get is a plain `char'.
Converting that `char' to a `void*' is suspect, to say the least.
Then again, you may have become lost in the thicket of types.
Perhaps the comment is correct and your original statement about
the type of `atoms->resname' is wrong. If `atoms->resname' is
actually a `char***', then `*(atoms->resname[0])' is indeed a
`char*' and not a `char', and this almost makes sense. (Of course,
the dubious "%x" is still a potential problem.)
[color=blue]
> (void) fprintf(stderr,"1g: %8x\n",(void *) *(atoms->resname[1]));
> (void) fprintf(stderr,"2g: %8x\n",(void *) *(atoms->resname[2]));
>
> atoms->resname= (char ***) &pnew_res_names;
> /* the explict cast doesn't seem to make any difference */[/color]
If `atoms->resname' is a `char**' as you say, the compiler
should issue a diagnostic for the type mismatch here.
[color=blue]
> /* char **, pointers to pointers to the name strings */
> (void) fprintf(stderr,"-1a: %8x\n",(void *) pnew_res_names);[/color]
Semi-legitimate, the "%x" being the only problem I see.
[color=blue]
> /* char *, pointers to the first characters in the name strings */
> (void) fprintf(stderr,"0a: %8x\n",(void *)pnew_res_names[0]);[/color]
Remember what I said above about indeterminate values? The
variable `pnew_res_names' points to some memory (assuming malloc()
succeeded), but the contents of that memory are "random garbage."
In other words, this call prints random garbage (if it prints
anything at all; the use of an indeterminate value produces
undefined behavior).
[color=blue]
> (void) fprintf(stderr,"1a: %8x\n",(void *)pnew_res_names[1]);
> (void) fprintf(stderr,"2a: %8x\n",(void *)pnew_res_names[2]);
> /* char **, pointers to pointers to the name strings */
> (void) fprintf(stderr,"0b: %8x\n",(void *)(atoms->resname[0]));[/color]
Here you imitate Wile E. Coyote by running off the edge of
a cliff in hot pursuit of Roadrunner. `pnew_res_names' points to
memory filled with random garbage, so here you fetch some of that
garbage and try to use it as the address of another piece of memory
which you try to fetch and print. All Hell breaks loose.
[color=blue]
> (void) fprintf(stderr,"1b: %8x\n",(void *)(atoms->resname[1]));
> (void) fprintf(stderr,"2b: %8x\n",(void *)(atoms->resname[2]));
> /* char *, pointers to the first characters in the name strings */
> (void) fprintf(stderr,"0c: %8x\n",(void *) *(atoms->resname[0]));
> (void) fprintf(stderr,"1c: %8x\n",(void *) *(atoms->resname[1]));
> (void) fprintf(stderr,"2c: %8x\n",(void *) *(atoms->resname[2]));
>
> When this code runs it emits this:
>
> 0e: 804c110 #so the original atoms->resname can be accessed by
> 0f: 804a6e8 #indices
> 1f: 804a750
> 2f: 804a75c
> 0g: 804b6e8
> 1g: 804b888
> 2g: 804b8b8
>
> -1a: 80e98e0 #location of pnew_res_names
> 0a: 804b6e8 #first points to first string
> 1a: 804b6f8 #second points to second string
> 2a: 804b708 #third points to third string
> 0b: 80e98e0 #atoms->resname[0] points to first spot in pnew_res_names
> 1b: 0 #second is broken, no pointer to 2nd string, but see 1a
> 2b: 0 #third is broken, no pointer to 3rd string, but see 2a
> 0c: 804b6e8 #correctly points to first string
> segfaults when it tries to reference address 0.
>
> In other words, the chunk of memory that was originally allocated
> and passed in as atoms->resname can be accessed by indices
> properly. The newly allocated region can also be accessed
> when referenced from pnew_res_names. However, it cannot be
> accessed via indices through atoms->resname after that is set
> to point to the memory block. The first value works
> properly, but the second and subsequent ones do not.[/color]
Presumably because no values have been stored in that block
of memory. Garbage in, garbage out.
[color=blue]
> All (reasonable) accesses to atoms->resname work before the memory
> block is reassigned. All (reasonable) accesses to pnew_res_names
> work before and after the memory block is reassigned. But mixed
> pointer/index accesses to the data in pnew_res_names does not work
> once it is linked to atoms->resname.[/color]
The memory block is not "reassigned," nor has any data been
placed in the memory addressed by `pnew_res_names', nor is that
memory somehow "linked" to the memory formerly addressed by
`atoms->resname'. You start with `atoms->resnames' pointing to
a chunk of memory we'll call Area A, filled with sensible data
(or so we imagine). You allocate another chunk of memory we'll
call Area 51, and cause `pnew_res_names' to point to it; this
memory is filled with random garbage. Then you change the
`atoms->resnames' pointer so it now points to Area 51 -- nothing
has happened to either memory block, and no data has been magically
[color=blue]
> Why? Or more specifically, is there some magical cast that can
> be used instead of (char ***) to make this work?[/color]
As far as I can see, magic is your only hope ;-) You've
got some code that (1) gets itself all snarled in a tangle of
types and can't keep track of what's what, and (2) mixes up the
pointer and the pointee and thus winds up reading uninitialized
memory. To fix (1) you might introducing a few typedefs to hide
some of the levels of pointer indirection -- I am usually averse
to typedef'fing aliases for pointer types, but when you go deeper
than two asterisks (and show evidence of confusion) it may be
justifiable. To fix (2) you need to step back and consider what
this newly-allocated memory block is supposed to do, what values
it is supposed to hold, and where it's supposed to get them.
--
Eric Sosman
esosman@acm-dot-org.invalid
