Pointer function returning false value

Question

I have written a function like this, when the parameter x is even it doesn't work as desired, for example if I type printf("%s",maxCharac(2)) in main it will print aa and an extra character next to it, but with an odd number it works properly.

char *maxCharac(int x)
{
    char *str=(char*)malloc(sizeof(char)*x);
    for(int i=0;i<x;i++)
    {            
        str[i]='a';
    }
    return str;   
}

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Apr 10 '17 at 14:19
You have to null terminate it. Add this line after the `for` loop and add `+ 1` to the `malloc` line. `str[x] = NULL;` — imreal, Apr 10 '17 at 14:19
You must null-termante your string, so you need an extra byte after your chars. — LPs, Apr 10 '17 at 14:20

LPs · Accepted Answer · 2017-04-10T14:38:32.980

C strings are NUL terminated, so

char *maxCharac(int x)
{
    char *str = malloc(x + 1);

    if (str != NULL)
    {
       for (int i = 0; i < x; i++)
       {
          str[i] = 'a';
       }
       str[i] = '\0';
    }

    return str;
}

As you can see:

You must leave room for a null terminator '\0' malloc(x + 1);
sizeof(char) is always 1 per standard
You must check malloc&co return value != NULL before use it.

Or, to avoid last instruction you can use calloc that zeros the allocated memory

char *maxCharac(int x)
{
    char *str = calloc(x + 1, 1);

    if (str != NULL)
    {
       for (int i = 0; i < x; i++)
       {
          str[i] = 'a';
       }
    }

    return str;
}

Last thing, as per function, the caller must check return value of function to ensure to not use the possible NULL pointer returned:

int main(void)
{
    char *str = maxCharac(2);

    if (str != NULL)
    {
        printf("Test: %s\n", str);
    }   
}

@SinanÜnür Thanks for your edits and sorry for my english ;) — LPs, Apr 10 '17 at 14:42

score 2 · Answer 2 · answered Apr 10 '17 at 14:21

2

You aren't allocating enough memory for your string as you need an extra character to null-terminate it, so you want to allocate one extra like this and also make sure that the last character is a NUL.

char *str=(char*)malloc(sizeof(char)*(x+1));
for(int i=0;i<x;i++){  
   str[i]='a';
}

str[x]='\0';

Without the extra NUL character at the end, you are experiencing undefined behaviour - your code will keep reading beyond the end of the string until it encounters a NUL character. That you are seeing one when x is odd at the right point to terminate your string is pure luck.

answered Apr 10 '17 at 14:21

Chris Turner

8,082
1
14
18

Strictly [tag:c] speaking the cast of `malloc` return is not required. – LPs Apr 10 '17 at 14:31
Don't cast the return value of `malloc`. `sizeof(char)` is ***by definition*** always 1. – Sinan Ünür Apr 10 '17 at 14:31
@LPs I know it's not required, but it was in the original code so I left it in and only corrected the parts that definitely were wrong – Chris Turner Apr 10 '17 at 14:33
[It is not just "not required"](http://c-faq.com/malloc/mallocnocast.html). – Sinan Ünür Apr 10 '17 at 14:37
@SinanÜnür it doesn't hurt to make things explicit in the code even if it'll always be 1 - really it should be `sizeof(*str)` as that way it ensures it's always the right type for `str` – Chris Turner Apr 10 '17 at 14:37

Pointer function returning false value

2 Answers2