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

`s[i] = "0";` should be `s[i] = '0';` so enable compiler warnings. — Weather Vane, Aug 29 '22 at 08:28
On an unrelated note, please don't implement your own string copying. Use `strcpy` instead. — Some programmer dude, Aug 29 '22 at 08:29
I've thought '0' is the zero-terminator so if I use this one the string ends at the first 0 @WeatherVane — Gabriel Burzacchini, Aug 29 '22 at 08:30
Also after you fill with zeros, the loop `for (i; i < n; i++)` to copy the string will not work very well, since it will start copying from index `i` (not `0`). — Some programmer dude, Aug 29 '22 at 08:31
And you could solve it using `snprintf` instead of manually copying characters: `snprintf(s, sum, "%0*u%s", zeros, 0, str)` — Some programmer dude, Aug 29 '22 at 08:36
You've re-invented `memset()` and `strcpy()`.... The entire 'longer' code could be replaced with `strdup()` if it's available in your libraries... Better coders use standard library calls instead of re-inventing buggy versions of the same functionality. You DO use `strlen()`, however! — Fe2O3, Aug 29 '22 at 09:04

Vlad from Moscow · Answer 1 · 2022-08-29T21:05:43.283

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

Minor: Could omit `result[i] = '\0';` and use `i < n;` --> `i <= n;`. — chux - Reinstate Monica, Aug 29 '22 at 19:02

score 1 · Answer 2 · answered Aug 29 '22 at 10:12

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.

OP used `size_t` for string sizing to handle all string lengths. This answer uses `int`, a lesser choice. — chux - Reinstate Monica, Aug 29 '22 at 18:59
OP's code does not cast the return of `malloc()`, yet this code does the [anti-pattern](https://stackoverflow.com/a/605858/2410359). — chux - Reinstate Monica, Aug 29 '22 at 19:04
@chux-ReinstateMonica `int` v. `size_t`... yes, an old habit. `cast return from malloc()`: my old compiler requires that. `max()` is defined in the first code block of this answer... There's a lot of old code (and many old coders) out there. Newbie's should learn that "it wasn't always done like it is today". The OP also used `const` whereas I omitted that... `:)` — Fe2O3, Aug 29 '22 at 20:57

score 0 · Accepted Answer · answered Aug 29 '22 at 08:55

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;
}

adding zeros before string

3 Answers3