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
CodePudding user response：
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.
CodePudding user response：
so you have 3 major problems in your code :
- it's
s[i] = '0';nots[i] = "0"; - it's
s[i] = str[i - zeros];nots[i] = str[i];as the value of the i will be 27 in your test case : so it make sense to says[27]because its size is about 41 but it doesn't make sense to saystr[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 usefor (; i < zeros; i )instead offor (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;
}
CodePudding user response：
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