2

The task should be simple, remove first and last characters.

https://www.codewars.com/kata/56bc28ad5bdaeb48760009b0/train/c

The function gets two parameter (dst as destination and src as source), and should return a modified string and assign to dst pointer (if I understood correctly).

My Answer looks correct to me, but here is my problem:

When the string has more then 9 characters, the modified string comes with some symbols.

char* remove_char(char* dst, const char* src){

  memmove(dst,src+1,strlen(src+1)-1);


  return dst;
}

Thanks in advance for your help :)

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
StackInTheFloor
  • 54
  • 8

3 Answers3

6

When doing this:

memmove(dst,src+1,strlen(src+1)-1);

You're correctly skipping the first and last character, but you end up with a string that has no NUL terminator (\0). You should add it by yourself before or after the memmove:

size_t len = strlen(src) - 2;
memmove(dst, src + 1, len);
dst[len] = '\0';

Of course, all of the above code assumes that dst was correctly allocated and can contain at least strlen(src) - 1 characters and that src has at least 2 characters.

If you also want to account for the edge case in which src is shorter than two characters:

size_t len = strlen(src);

if (len < 2) {
    *dst = '\0';
} else {
    memmove(dst, src + 1, len - 2);
    dst[len - 2] = '\0';
}

return dst;

Note: you might have to #include <stddef.h> to use size_t.

Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128
  • So basically String ( declared as pointer) behaves like an array ? – StackInTheFloor Jan 31 '20 at 20:02
  • 1
    @michael1995 a string *is* an array. The only difference between a string and a normal character array is that a string is always terminated with the NUL-terminator `\0`. – Marco Bonelli Jan 31 '20 at 20:04
  • size_t is defined in ``, no need for `` – chqrlie Jan 31 '20 at 20:42
  • @chqrlie isn't it the opposite? I.E. string.h includes stddeff.h which is where size_t is defined. https://stackoverflow.com/questions/1119370/where-do-i-find-the-definition-of-size-t – Marco Bonelli Jan 31 '20 at 20:48
  • 1
    There's no need to include `` for `size_t` because `` must have been included to use `memmove()` and `strlen()`, and `string.h>` also defines `size_t`. – Jonathan Leffler Jan 31 '20 at 23:07
  • @JonathanLeffler try running the program using the link provided by OP and you'll see that stddef is needed. I added that clarification only because of this. Sure it'a already included in string.h but I don't know what's the deal with that online compiler. – Marco Bonelli Jan 31 '20 at 23:59
  • @MarcoBonelli: `size_t` is defined in multiple standard include files: ``, ``, ``, ``, ``, ``, ``, and many other system specific headers. If the code uses `memmove`, it should include `` hence `size_t` will be defined. Technically, including just `` will cause `size_t` to be defined and calling `memmove` without a prototype might not prevent compilation, but it is highly advisable to always include the proper header files and to enable all warnings. – chqrlie Feb 02 '20 at 10:29
  • @MarcoBonelli: testing with the online compiler indeed shows the above code compiles with either `` or ``. Changing the code to `memmove((long)dst, src + 1, (char*)(len - 2));` compiles fine with both header files and passes the tests, proving that the online compiler is configured with very lenient settings, only failing on errors and suppressing basic warnings. A very bad choice for beginners. One can choose which compiler to use, but cannot add any command line flags such as `-W -Wall -Werror` which should be the default. – chqrlie Feb 02 '20 at 10:39
3

This call

memmove(dst,src+1,strlen(src+1)-1);

does not make a string in the character array pointed to by the pointer dst because the terminating zero is not copied and the destination array can not be zero-initialized.

Also the expression strlen( src + 1 ) - 1 can invoke undefined behavior.

And there is no sense to use memmove provided that the character arrays are not overlapped.

Here is a demonstrative program that shows how the task can be performed.

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

char * remove_char( char * restrict dst, const char * restrict src )
{
    size_t n = strlen( src );

    n = n < 2 ? 0 : n - 2;

    if ( n != 0 )
    {
        memcpy( dst, src + 1, n );
    }

    dst[n] = '\0';

    return dst;
}

int main(void) 
{
    enum { N = 10 };
    char dst[N];

    printf( "\"%s\"\n", remove_char( dst, "" ) );
    printf( "\"%s\"\n", remove_char( dst, "1" ) );
    printf( "\"%s\"\n", remove_char( dst, "12" ) );
    printf( "\"%s\"\n", remove_char( dst, "121" ) );

    return 0;
}

The program output is

""
""
""
"2"
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

Your code has multiple problems:

  • it fails because you do not set the null terminator at the end of the destination array.
  • it is brittle because you do not test the length of the source string to be at least 2 bytes, causing undefined behavior on shorter strings.
  • it compiles by chance because you do not include <string.h> and thus call functions without a proper prototype. The prototypes inferred by the compiler are incorrect.

The Solution posted on the codewars online compiler should compile as a stand alone file. It must include the relevant files, such as <string.h> if you use memmove() or strlen().

Here is a corrected solution:

#include <string.h>

char *remove_char(char *dst, const char *src) {
    size_t len = strlen(src);
    if (len >= 2) {
        memmove(dst, src + 1, len - 2);
        dst[len - 2] = '\0';
    } else {
        *dst = '\0';
    }
    return dst;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189