Atlas Chiew
Atlas Chiew

Reputation: 31

Unexpected array behaviour with use of sprintf in C

I am facing weird behaviour after apply sprintf: list[0] seems just gone away and result of strlen is 0. Then I try to apply strcpy, strlen meets expectation and returns 3. So my question is why sprintf will erase my list[0], how do I recover value of list[0] if I insist to apply sprintf? thanks in advance.

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

void main() {
    char list[5][7] = { "One", "Two", "Three", "Four", "Five" };    
    char item[7];

    int i = 0;

    for (i = 0; i < 5; i++) {
        sprintf(item, "%-7s", list[i]);
        //strcpy(item, list[i]);
    }

    printf("%d", strlen(list[0]));
}

Upvotes: 1

Views: 653

Answers (4)

chqrlie
chqrlie

Reputation: 145317

Your code has undefined behavior because %-7s requires at least 8 bytes in the destination array: 7 for the left aligned string plus one for the null terminator. Note that if any of the strings were longer than 7 bytes, even more space would be required in the destination array.

This mistake might explain the observed behavior: sprintf writes 8 bytes to item, ie one byte too many, and if the array list starts in memory just after the end of item, its first byte would be overwritten with a null bytes causing list[0] to to be truncated and become an empty string with a length of 0.

You should use snprintf that prevents buffer overflow and make your array at least 8 bytes long.

Note also that main has a return type of int.

Here is a modified version:

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

int main() {
    char list[5][7] = { "One", "Two", "Three", "Four", "Five" };    
    char item[8];

    for (int i = 0; i < 5; i++) {
        snprintf(item, sizeof item, "%-7s", list[i]);
    }
    printf("%d\n", strlen(list[0]));
    return 0;
}

Upvotes: 7

August Karlstrom
August Karlstrom

Reputation: 11377

As pointed out in other replies, the field width in sprintf mustn't exceed the length of the array minus one. The real solution is to not hard code any values which can be calculated from previous declarations. The program below shows a more robust version:

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

#define LEN(arr) ((int) (sizeof (arr) / sizeof (arr)[0]))

int main(void)
{
    char list[][7] = { "One", "Two", "Three", "Four", "Five" }; 
    char item[LEN(list[0])];

    int i = 0;

    for (i = 0; i < LEN(list); i++) {
        sprintf(item, "%-*s", LEN(list[0]) - 1, list[i]);
    }

    printf("%d", strlen(list[0]));
    return 0;
}

Now, say you want to change the length of the character arrays. Then you only need to change the value 7 to something else and the rest will follow.

See also https://en.wikipedia.org/wiki/Magic_number_(programming)#Unnamed_numerical_constants

Upvotes: 1

Welgriv
Welgriv

Reputation: 833

As other said:

At this line: sprintf(item, "%-7s", list[i]); the item char array is supposed to be at least 8 bytes long to hold 7 characters + the end string byte \0.

Moreover sprintf is dangerous you should use snprintf instead.

Upvotes: 5

Nima
Nima

Reputation: 381

I think it is the "%-7s" that's causing the problem

Since the strings are 6 characters long, isn't it better to use "%-6s" instead? the problem seems to get solved that way.

Edit: Or as mentioned by @Welgriv in the comments, set size of the char arrays to 8 to have 7 characters

Upvotes: 1

Related Questions