adding zeros before string

Question

zfill algorithm is supposed to work as follows:

• zfill function accepts two parameters, a string and a number,
• if string length is >= the number, then it doesn't have to add anything, and it returns a copy to the string,
• else, malloc enough space and add zeros before the string.

I'm trying to understand why is this solution not correct, it has two warnings: 1st warning:

 for (i; i < zeros; i++) {
            s[i] = "0"; 
        }

"=": char differs in level of indirection from char[2]

2nd warning:

for (i; i < n; i++) {
            s[i] = str[i]; 
        }

buffer overrun while writing to s

---

    char* zfill(const char* str, size_t n) {
        if (str == NULL) {
            return NULL; 
        }
        char* s; 
        size_t length = strlen(str); 
        if (length >= n) {
            //it doesn't have to add anything, just malloc and copy the string
            size_t sum = length + 1u; 
             s = malloc(sum); 
            if (s == NULL) {
                return NULL; 
            }
            for (size_t i = 0; i < length; i++) {
                s[i] = str[i]; 
            }
            s[sum] = 0; 
        }
        else {
            // add zeros before strings
            size_t zeros = n - length; 
            size_t sum = n + 1u; 
             s = malloc(sum); 
            if (s == NULL) {
                return NULL; 
            }
            size_t i = 0;
            for (i; i < zeros; i++) {
                s[i] = "0"; 
            }
            for (i; i < n; i++) {
                s[i] = str[i]; 
            }
            s[sum] = 0; 
        }
        return s; 
    }
    
    int main(void) {
        char str[] = "hello, world!"; 
        size_t n = 40; 
    
        char* s = zfill(str, n); 
    
    
        free(s); 
        return 0; 
    }

EDIT: I've solved the problem this way:

char* zfill(const char* str, size_t n) {
    if (str == NULL) {
        return NULL; 
    }
    char* s; 
    size_t length = strlen(str); 
    if (length >= n) {
        //it doesn't have to add anything, just malloc and copy the string
        size_t sum = length + 1u; 
         s = malloc(sum); 
        if (s == NULL) {
            return NULL; 
        }
        for (size_t i = 0; i < length; i++) {
            s[i] = str[i]; 
        }
        s[sum-1] = 0; 
    }
    else {
        // add zeros before strings
        size_t zeros = n - length; 
        size_t sum = n + 1u; 
         s = malloc(sum); 
        if (s == NULL) {
            return NULL; 
        }
        size_t i = 0;
        for (i; i < zeros; i++) {
            s[i] = '0'; 
        }
        for (size_t j = 0; i < n; j++) {
            s[i++] = str[j]; 
        }
        s[sum-1] = 0; 
    }
    return s; 
}

and it works, but I don't know why I have this warning:

for (i; i < zeros; i++) {}

statement with no effect

but when I've debugged I've noticed that this statement has an effect, because it correctly copies the correct number of zeros. I don't know why I have this warning

Answer 1

This code snippet

         size_t sum = length + 1u; 
         s = malloc(sum); 
         //...
         s[sum] = 0; 

accesses memory outside the allocated character array because the valid range of indices is [0, sum). You need to write at least like

         s[length] = 0; 

In this code snippet

 for (i; i < zeros; ++) {
            s[i] = "0"; 
        }

the expression s[i] represents a single object of the type char while on the right-hand side there is a string literal that as an expression has the type char *. You need to write at least

s[i] = '0';

using the integer character constant instead of the string literal.

In this code snippet

        size_t i = 0;
        for (i; i < zeros; i++) {
            s[i] = "0"; 
        }
        for (i; i < n; i++) {
            s[i] = str[i]; 
        }

as the length of the string str can be less than n then this for loop

        for (i; i < n; i++) {
            s[i] = str[i]; 
        }

accesses memory outside the string str.

Pay attention to that your function has redundant code. It can be written simpler.

The function can look for example the following way as shown in the demonstration program below.

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

char * zfill( const char *s, size_t n ) 
{
    char *result = NULL;

    if ( s != NULL )
    {
        size_t len = strlen( s );

        n = len < n ? n : len;

        result = malloc( n + 1 );

        if ( result )
        {
            size_t i = 0;

            size_t m = len < n ? n - len : 0;
            
            for ( ; i < m; i++ )
            {
                result[i] = '0';
            }

            for ( ; i < n; i++ )
            {
                result[i] = s[i - m];
            }

            result[i] = '\0';
        }
    }

    return result;
}

int main( void ) 
{
    const char *s = "Hello";
    size_t n = 10;

    char *result = zfill( s, n );

    if ( result ) puts( result );

    free( result );
}

The program output is

00000Hello

Or as @Some programmer dude pointed to in his comment you can use the standard C function snprintf that alone performs the task. For example

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

char * zfill( const char *s, size_t n ) 
{
    char *result = NULL;

    if ( s != NULL )
    {
        size_t len = strlen( s );

        n = len < n ? n : len;

        result = malloc( n + 1 );

        if ( result )
        {
            int m = len < n ? n - len : 0;
            snprintf( result, n + 1, "%.*d%s", m, 0, s );
        }
    }

    return result;
}

int main( void ) 
{
    char *p = zfill( "Hello", 5 );
    if ( p ) puts( p );
    free( p );

    p = zfill( "Hello", 10 );
    if ( p ) puts( p );
    free( p );
}

The program output is

Hello
00000Hello

Answer 2

SO is a place of learning.

When first dealing with a coding challenge, it's best to take time to work out what's needed before starting to write code. Below is a working version of zfill() (along with a main() that tests it.) Read through the comments. The only thing new here is memset().

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

// A trivial "helper function" determines the max of two values
int max( int a, int b ) { return a > b ? a : b; }

char *zfill( char *str, int minLen ) {
    // Determine length of arbitrary string
    int len = strlen( str );

    // Determine max of length v. minimum desired
    int allocSize = max( minLen, len );

    // allocate buffer to store resulting string (with '\0')
    char *obuf = (char*)malloc( allocSize + 1 );
    /* omitting test for failure */

    // determine start location at which to copy str
    int loc = len <= minLen ? minLen - len : 0;
    if( loc > 0 )
        // fill buffer with enough 'zeros'
        memset( obuf, '0', allocSize ); // ASCII zero!

    // copy str to that location in buffer
    strcpy( obuf + loc, str );

    // return buffer to calling function
    return obuf;
}

int main() {
    // collection of strings of arbitrary length
    char *strs[] = { "abc", "abcdefghijkl", "abcde", "a", "" };

    // pass each one to zfill, print, then free the alloc'd buffer.
    for( int i = 0; i < sizeof strs/sizeof strs[0]; i++ ) {
        char *cp = zfill( strs[i], 10 );
        puts( cp );
        free( cp );
    }

    return 0;
}

Output:

0000000abc
abcdefghijkl
00000abcde
000000000a
0000000000

Here's zfill() without the comments:

char *zfill( char *str, int minLen ) {
    int len = strlen( str );

    int allocSize = max( minLen, len );
    char *obuf = (char*)malloc( allocSize + 1 );
    /* omitting test for failure */

    int loc = len <= minLen ? minLen - len : 0;
    if( loc > 0 )
        memset( obuf, '0', loc ); // ASCII zero!

    strcpy( obuf + loc, str );

    return obuf;
}

You don't want to spend your time staring at lines and lines of code. Fill your quiver with arrows that are (proven!) standard library functions and use them.

I've omitted, too, the test for zfill being passed a NULL pointer.

Answer 3

so you have 3 major problems in your code :

• it's s[i] = '0'; not s[i] = "0";
• it's s[i] = str[i - zeros]; not s[i] = str[i]; as the value of the i will be 27 in your test case : so it make sense to say s[27] because its size is about 41 but it doesn't make sense to say str[27] as its size is only about 13 in your test case , so you had to map the value 27 of i to the value 0 to be convenient to use with str
• i is deprecated in first part here for (i; i < zeros; i++) , so use for (; i < zeros; i++)instead of for (i; i < zeros; i++) , but it will not cause any problem if you keep it.

---

and here is the full edited code :

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

char* zfill(const char* str, size_t n) {
    if (str == NULL) {
        return NULL;
    }
    char* s;
    size_t length = strlen(str);
    if (length >= n) {
        //it doesn't have to add anything, just malloc and copy the string
        size_t sum = length + 1u;
        s = malloc(sum);
        if (s == NULL) {
            return NULL;
        }
        for (size_t i = 0; i < length; i++) {
            s[i] = str[i];
        }
        s[sum] = 0;
    }
    else {
        // add zeros before strings
        size_t zeros = n - length;
        size_t sum = n + 1u;
        s = malloc(sum);
        if (s == NULL) {
            return NULL;
        }
        size_t i = 0;
        for (; i < zeros; i++) {
            s[i] = '0';
        }
        for (; i < n; i++) {
            s[i] = str[i - zeros];
        }
        s[sum] = 0;
    }
    return s;
}

int main(void) {
    char str[] = "hello, world!";
    size_t n = 40;

    char* s = zfill(str, n);
    printf("%s\n", s);

    free(s);
    return 0;
}