16

I'm having trouble printing a member of a struct that is returned from a function:

#include <stdio.h>

struct hex_string
{
    char a[9];
};

struct hex_string to_hex_string_(unsigned x)
{
    static const char hex_digits[] = "0123456789ABCDEF";
    struct hex_string result;
    char * p = result.a;
    int i;
    for (i = 28; i >= 0; i -= 4)
    {
        *p++ = hex_digits[(x >> i) & 15];
    }
    *p = 0;
    printf("%s\n", result.a);   /* works */
    return result;
}

void test_hex(void)
{
    printf("%s\n", to_hex_string_(12345).a);   /* crashes */
}

The printf call inside to_hex_string_ prints the correct result, but the printf call inside test_hex crashes my program. Why exactly is that? Is it a lifetime issue, or is it something else?

When I replace the printf call with puts(to_hex_string_(12345).a), I get a compiler error:

invalid use of non-lvalue array

What's going on here?

Keith Thompson
  • 254,901
  • 44
  • 429
  • 631
fredoverflow
  • 256,549
  • 94
  • 388
  • 662
  • 1
    @LuchianGrigore: That's one possible symptom of undefined behavior. – Keith Thompson Nov 01 '11 at 08:24
  • Do you have `#include ` at the top of your source file? And why the trailing underscore on the function name? (It's legal, but odd.) – Keith Thompson Nov 01 '11 at 18:50
  • @KeithThompson: Yes, `stdio.h` is included. Trailing underscores are my personal convention for throwaway toy functions if I already have a stable function with the same name. – fredoverflow Nov 01 '11 at 19:02
  • I've added `#include ` to the question. There are cases (not this one) where a missing `#include` directive could be the cause of the problem. – Keith Thompson Nov 01 '11 at 19:07

3 Answers3

18

There is a rule in C which seldom comes into effect, which states:

If an attempt is made to modify the result of a function call or to access it after the next sequence point, the behavior is undefined. (C99 §6.5.2.2)

In this case, there is a sequence point after the arguments to printf() are evaluated and before the printf() function itself executes. The pointer you pass to printf() is a pointer to an element of the return value itself - and when printf() tries to access the string through that pointer, you get your crash.

This issue is hard to run into, because a function value isn't an lvalue so you can't directly take a pointer to it with &.

caf
  • 233,326
  • 40
  • 323
  • 462
  • So how come the `printf` call crashes, whereas the `puts` call fails to compile? – fredoverflow Nov 01 '11 at 10:49
  • @FredOverflow: The difference in gcc behaviour appears to be a result of the function prototype for `puts()` (you get the same warning if you try an assignment like `char *x = to_hex_string(1).a`). I believe it to be a bug, because the undefined behaviour only kicks in once the array is accessed through that pointer value; merely copying the pointer value does not seem to contradict any constraint. It is probably not an important one, though. – caf Nov 01 '11 at 12:51
  • I note also that the gcc warning disappears if you use `&to_hex_string(1).a[0]`, underscoring the fragility surrounding that warning. – caf Nov 01 '11 at 13:06
  • I'm curious: I can't find those words in C99 §6.5.2.2 in the original version of the standard, nor in any of the 3 technical corrigenda. It also doesn't exist in the C11 standard in section §6.5.2.2. – Jonathan Leffler Jul 05 '16 at 03:14
  • In C11, §6.2.4 ¶8 says _A non-lvalue expression with structure or union type, where the structure or union contains a member with array type (including, recursively, members of all contained structures and unions) refers to an object with automatic storage duration and temporary lifetime.36) Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends when the evaluation of the containing full expression or full declarator ends. Any attempt to modify an object with temporary lifetime results in undefined behavior._ – Jonathan Leffler Jul 05 '16 at 03:14
  • The equivalent words don't exist in the corresponding section of C99 (also §6.2.4 — there is no ¶8 though). I may still just have missed them. – Jonathan Leffler Jul 05 '16 at 03:17
  • @JonathanLeffler: It's the last sentence of §6.5.2.2 ¶5 in C99 TC3. – caf Jul 05 '16 at 03:24
  • Ah, found it — it was present in ¶5 in the original standard, too. I'm not sure how I missed it. The rules in C11 are somewhat different, it would seem — that sentence seems to be missing from §6.5.2.2, and the nearest I've found is §6.2.4 ¶8, already quoted. – Jonathan Leffler Jul 05 '16 at 04:31
  • Yes, one of the changes in C11 was to introduce objects of temporary lifetime. The question is only really applicable to pre-C11 environments I think. – caf Jul 05 '16 at 09:08
13

You've managed to run into a fairly obscure corner case of the language.

An expression of array type, in most contexts, is implicitly converted to a pointer to the first element of the array; the exceptions are when the expression is the operand of a unary & operator, when it's the operand of a unary sizeof operator, and when it's a string literal in an initializer used to initialize an array object. None of these exceptions applies here.

But there's an implicit assumption in that conversion: the pointer is to the first element of the array object.

Most array expressions -- almost all of them, in fact -- refer to some array object, such as a declared array variable, an element of a multidimensional array, and so forth. Functions can't return arrays, so you can't get a non-lvalue array expression that way.

But as you've seen, a function can return a struct that contains an array -- and there's no object associated with the array expression to_hex_string_(12345).a.

The new ISO C11 standard addresses this by adding new wording to the section describing storage durations. The N1570 draft, section 6.2.4p8, says:

A non-lvalue expression with structure or union type, where the structure or union contains a member with array type (including, recursively, members of all contained structures and unions) refers to an object with automatic storage duration and temporary lifetime. Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends when the evaluation of the containing full expression or full declarator ends. Any attempt to modify an object with temporary lifetime results in undefined behavior.

In effect, this says that the returned value from your function (unlike most function results) is the value of a temporary object, allowing the decay of its array member to give you a (temporarily) valid pointer.

But until compilers fully support the new C standard (which won't be for some years), you'll just have to avoid referring to array members of returned structures.

Keith Thompson
  • 254,901
  • 44
  • 429
  • 631
  • Aha, so C doesn't have C++'s notion of temporary objects (yet). Interesting choice to make them "const", that isn't the case in C++. – fredoverflow Nov 01 '11 at 08:21
  • So how come the `printf` call crashes, whereas the `puts` call fails to compile? – fredoverflow Nov 01 '11 at 10:49
  • @FredOverflow: It may well be a gcc bug. Using gcc 4.5.2, I get a warning on the `printf` call: `warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘char[9]’` (so apparently it's not doing the usual array-to-pointer conversion). On the `puts` call, I get `error: invalid use of non-lvalue array`, as you did. I can't think of any good reason for them to behave *that* differently. But the behavior is undefined, so you should just avoid doing that. – Keith Thompson Nov 01 '11 at 18:49
  • 2
    @FredOverflow: It's not actually "const". Attempting to modify it has undefined behavior, but it's not `const`-qualified. It's similar to C's treatment of string literals. – Keith Thompson Dec 19 '11 at 21:49
-1

The problem you're facing is that : the variable result which is return is a local variable of the function _to_hex_string, that's means it is deleted at the end of the fonction call. so when you try to check it in the fonction test_hex it is no available any more.

To solve your problem you can deal with pointer.

here is your code modify

struct hex_string
{
    char a[9];
};

struct hex_string * to_hex_string_(unsigned x) // here you return a pointer
{
    static const char hex_digits[] = "0123456789ABCDEF";
    struct hex_string result;

    result = (struct hex_string *) malloc(sizeof(struct hex_string));
    char * p = result->a;
    int i;

    for (i = 28; i >= 0; i -= 4)
    {
        *p++ = hex_digits[(x >> i) & 15];
    }

    *p = 0;
    printf("%s\n", result->a);   /* works */
    return result;
}

void test_hex(void)
{
    printf("%s\n", to_hex_string_(12345)->a);  /* works */
}

Have I nice day

Fopa Léon Constantin
  • 11,863
  • 8
  • 48
  • 82