CODEWITHSUNDEEP
cc-strings
StackInTheFloor
StackInTheFloor

Reputation: 64

Remove first and last character from a string

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 :)

Upvotes: 2

Views: 819

Answers (3)

chqrlie
chqrlie

Reputation: 144550

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

Upvotes: 0

Marco Bonelli
Marco Bonelli

Reputation: 69276

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.

Upvotes: 6

Vlad from Moscow
Vlad from Moscow

Reputation: 310910

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"

Upvotes: 3

Related Questions