1

I have some C code that is supposed to call a function and return a string. Somewhere I read this is difficult due to memory allocation not being constant or something? Anyway, I was told I should rather return the pointer to the string. However, I can't explain why printf sometimes produces gibberish, and sometimes not:

#include <stdio.h> 
#include <string.h>
#include <stdlib.h>

char * execTasks() {

    int d, m, y;
    char str[6] = {};

    d = 10; m = 5; y = 18;

    memset(&str, 0, sizeof(str));
    sprintf(str, "%02d%02d%02d", d, m, y);
    printf("Works fine: %s\n", str); // Works fine even with additions

    char *s_ptr = str;
    return s_ptr; 
}

int main() {

    char * str;

    str = execTasks();

    printf("%s", str); // works fine!
    printf("%s\n", str); // produces gibberish!
    printf("%s,%s", str, str); // also gibberish!

    return 0;
}

Can somebody explain to me why printf("%s", str); works fine, while printf("%s\n", str); and variations print out gibberish?

Or am I doing something wrong with returning the pointer? Essentially the function should return a date as string in the format DDMMYY. I need this string later in the main function to append it to another string.

Yakov Galka
  • 70,775
  • 16
  • 139
  • 220
TJJ
  • 155
  • 10
  • 1
    `"%02d%02d%02d"` -- NO room for *nul-character*.... `char str[7]` (minimum) required, but *don't skimp on buffer size*, 8 or 16 is sufficient. – David C. Rankin May 25 '19 at 01:14
  • You are absolutely right. Been some time since I've been writing C code. Added `str[6] = '\0'`. What do you mean with buffer size though? – TJJ May 25 '19 at 14:25
  • 1
    The size of the array. There isn't any since is trying to set it exactly at 6 or 7 as the stack will be 4 or 8-byte aligned. May as well use 8-bytes, and as a general rule leaving only a single-byte margin wouldn't support a later simple change of `"%02d%02d%04d"`. Don't skimp on buffer size, use `char str[16];` (or `32`, etc..) In other words, so long as you are not talking about 200K size, if you think you know what the maximum array (buffer) size needed is -- double it. I'd rather have 100,000 bytes too many, than 1-byte too few. – David C. Rankin May 25 '19 at 19:55
  • I understand. But in this case it's totally predictable. DDMMYY, 6 chars. This is hardcoded in the GPS chip and international standard (NMEA0183). Yeah, don't ask me why they decided for 2-digit years instead of 4...it's also an embedded device, so keeping resources low is kinda mandatory. – TJJ May 25 '19 at 20:22
  • You run across those every once in a while. I guess as long as they know what century it is from other means, a 2-digit date is all that is needed. (and when you are minimizing satellite traffic, you probably look for any byte you can save) – David C. Rankin May 26 '19 at 00:02
  • True for knowing the century. But it's not the satellite that is using this format (GPS doesn't even know years, it counts only 1024 weeks and the seconds into the week), NMEA0183 is a standard for what the GPS or any other SATNAV receiver outputs to attached devices. So data has already been received and processed Earth-side. – TJJ May 26 '19 at 00:17
  • 1
    That make more sense. My experience with space-to-ground telemetry was limited to the shuttle downlink through Whitesands, NM and the processing at Goddard SFC in MD. – David C. Rankin May 26 '19 at 00:26

1 Answers1

3

The array char str[6] is local to the execTasks function. After execTasks returns, that memory is reclaimed and can be used by other code. It happens that the last two printfs in main use that memory and corrupt the string, whereas the first printf in main doesn't. However, this code is still invalid (as in Undefined Behavior) as it can also produce gibberish or crash in the right circumstances (e.g. a different compiler, architecture, or standard library implementation).

To fix that you shall not return pointers to local variables, and instead either use a dynamically allocated copy of the string:

char * execTasks() {
    int d, m, y;
    char str[7] = {};
    // ...
    return strdup(str); // mallocs a copy of str
}

int main() {
    char * str = execTasks();
    printf("%s,%s", str, str);
    free(str); // reclaim memory
}

Or use a variable that is local to main:

int execTasks(char *str, size_t size) {
    int d, m, y;
    d = 10; m = 5; y = 18;
    return snprintf(str, size, "%02d%02d%02d", d, m, y);
}

int main() {
    char str[7];
    execTasks(str, sizeof(str));
    printf("%s,%s", str, str);
}

Here execTasks returns the amount of characters (not including the null terminator) that would be needed to hold the result. It can be used to detect when the provided buffer is too small, though I don't do that in this simple example (if it's too small the string will simply be truncated).

Notice also that the amount of the memory required for the string is seven chars, because you need an extra byte for the null terminator.

Yakov Galka
  • 70,775
  • 16
  • 139
  • 220
  • Thx for the explanation. Got a bit rusty with all the pointers and memory allocation etc. I decided to go with the first example. Works like a charm. Thanks a lot again! – TJJ May 25 '19 at 14:26