Is this legal C code?

[Jim Ford]

I have the following code:

A * F(B * x)
{
A * y = (A *) *x->data ;

return y ;
}

Is this legal? Do you need more information about the details of the A
and B structures in order to assess this?

This is just the skeleton of some code (removing details irrelevant here)
that I have. The code works as expected, but that

return y ;

line worries me. Will the value of y after F returns be as expected under
all circumstances?

 

[Richard Heathfield]

I have the following code:

A * F(B * x)
{
A * y = (A *) *x->data ;

return y ;
}

Is this legal?

It depends on the nature of A and B. The cast isn't a good sign. If it's not
necessary, why is it there? And if it /is/ necessary, you probably didn't
want to do that conversion anyway.

Do you need more information about the details of the A
and B structures in order to assess this?
Yes.


This is just the skeleton of some code (removing details irrelevant here)
that I have. The code works as expected, but that

return y ;

line worries me. Will the value of y after F returns be as expected under
all circumstances?

No. For example, x could be NULL, in which case the behaviour of the code is
undefined.

 

[Jim Ford]

It depends on the nature of A and B. The cast isn't a good sign. If it's
not necessary, why is it there? And if it /is/ necessary, you probably
didn't want to do that conversion anyway.


Yes.

OK. We have the following:

typedef struct {
/* Some irrelevant fields */
} A ;

typedef struct {
/* Some fields */
char ** data ;
/* More fields */
} B ;

I know that the data field is a pointer to a sequence of pointers to data
arranged as in A. However, as can be seen above, the data field is a char
**, and changing this is beyond my purview. Hence the need for the cast,
in order to eliminate compiler warnings.

No. For example, x could be NULL, in which case the behaviour of the
code is undefined.

The assumption here is that everything concerning x is fine when entering
F, therefore implying that, when initialized inside F(), y has the right
value. The question is, is the return value of F(), as constructed here,
guaranteed to be same as the value of y as initialized in F()?

 

[Richard Heathfield]

OK. We have the following:

typedef struct {
/* Some irrelevant fields */
} A ;

typedef struct {
/* Some fields */
char ** data ;
/* More fields */
} B ;

I know that the data field is a pointer to a sequence of pointers to data
arranged as in A. However, as can be seen above, the data field is a char
**, and changing this is beyond my purview. Hence the need for the cast,
in order to eliminate compiler warnings.

That is not what casts are for. Your compiler is telling you "No! Bad!" and
the cast is the moral equivalent of you putting your hands over your ears
and saying "Not listening! Not listening! Na-na-na!"

Casts are almost always wrong.

The assumption here is that everything concerning x is fine when entering
F, therefore implying that, when initialized inside F(), y has the right
value.

That's an unfortunate assumption, since (unless I'm very much mistaken) the
initialisation invokes undefined behaviour. If you want to use a pointer
for "parking" an object of unknown type, why not use void * rather than
char *?

The question is, is the return value of F(), as constructed here,
guaranteed to be same as the value of y as initialized in F()?

No. As far as I can tell, you are invoking undefined behaviour by assigning
a pointer value to an object that is of a type not compatible with the type
of that pointer value.

 

[Richard Bos]

typedef struct {
/* Some irrelevant fields */
} A ;

typedef struct {
/* Some fields */
char ** data ;
/* More fields */
} B ;

I know that the data field is a pointer to a sequence of pointers to data
arranged as in A. However, as can be seen above, the data field is a char
**, and changing this is beyond my purview.

Ugly. Your best bet would be to find someone with a more extensive
purview, and nag them until they change it.

The assumption here is that everything concerning x is fine when entering
F, therefore implying that, when initialized inside F(), y has the right
value.

That assumption is not one that you can depend on in ISO C. If data were
a void *, or AFAIK even a char *, you could, but not as it is.
However...

The question is, is the return value of F(), as constructed here,
guaranteed to be same as the value of y as initialized in F()?

....if your assumption _is_ correct, then by the time you reach the
return statement, y has an ordinary struct pointer value, and struct
pointers are ordinary objects which, unlike arrays, can be returned by
value from a function.
Do note that the return value from F() now points inside *x, so the
validity of that return value depends on the life time of the object
which x pointed at.

Richard

 

[Jim Ford]

That is not what casts are for. Your compiler is telling you "No! Bad!"
and the cast is the moral equivalent of you putting your hands over your
ears and saying "Not listening! Not listening! Na-na-na!"
Casts are almost always wrong.

I don't dispute that. However, either I have the cast or else I must live
with compiler warnings. I have no control over the the definitions of the
A and B structures.

That's an unfortunate assumption, since (unless I'm very much mistaken)
the initialisation invokes undefined behaviour. If you want to use a
pointer for "parking" an object of unknown type, why not use void *
rather than char *?

Because I can't. Like I hinted to above, A and B are given to me by a
third party.

No. As far as I can tell, you are invoking undefined behaviour by
assigning a pointer value to an object that is of a type not compatible
with the type of that pointer value.

That would be true in general, but not in the case that occupies me. I
know that there is no type incompatibility, because of the way the code is
being used. I agree with you in the general case, but that is not what I
am looking into here. Still, your comments are much appreciated.

 

[Jim Ford]

Ugly. Your best bet would be to find someone with a more extensive
purview, and nag them until they change it.

Well, yes; however, I don't have the time, stamina or desire to go to a
multi-million dollar company and tell them "Hey, your code sucks here!
Please change it."

I have to deal with such code, and that's it.

That assumption is not one that you can depend on in ISO C. If data were
a void *, or AFAIK even a char *, you could, but not as it is.
However...


...if your assumption _is_ correct, then by the time you reach the
return statement, y has an ordinary struct pointer value, and struct
pointers are ordinary objects which, unlike arrays, can be returned by
value from a function.

OK. This is more like what was plaguing me here.

Do note that the return value from F() now points inside *x, so the
validity of that return value depends on the life time of the object
which x pointed at.

All right, thanks; this is the answer I was looking for.

 

[James Hu]

I don't dispute that. However, either I have the cast or else
I must live with compiler warnings. I have no control over the the
definitions of the A and B structures.

Use a temporary void * variable?

-- James