HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Interleave two strings

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
twointerleavestrings

Problem

This program is designed to interleave two strings, which are input as command line arguments. I'd appreciate any feedback as I am new to C and self-taught.

Edit: Since this is an exercise in becoming more familiar with the intricacies of strings and memory allocation, the output should be stored before being printed.

#include   // printf
#include  // malloc, EXIT_FAILURE/SUCCESS
#include  // strlen

int main(int argc, char const *argv[])
{
  // Expecting: exe name, str_a, str_b
  if (argc != 3)
  {
    return EXIT_FAILURE;
  }

  size_t str_a_len = strlen(argv[1]);
  size_t str_b_len = strlen(argv[2]);
  if (str_a_len != str_b_len)
  {
    return EXIT_FAILURE;
  }

  // +1 for NULL byte
  size_t str_c_len = str_a_len + str_b_len + 1;
  char * str_c = malloc(str_c_len);

   // No free memory
  if (strlen(str_c) == 0)
  {
    return EXIT_FAILURE;
  }

  // Zip letters, AAA BBB => ABABAB
  for (int i = 0, n = str_c_len; i < n; ++i)
  {
    str_c[i] = (i % 2 == 0) ? argv[1][i / 2] : argv[2][i / 2];
  }

  printf("%s\n", str_c);

  return EXIT_SUCCESS;
}


I am compiling with:

gcc *.c -Wall -g -o zip.exe

Solution

You allocate a memory block:

char * str_c = malloc(str_c_len);


and then pass it to the strlen():

if (strlen(str_c) == 0)
    ....


Apart from the str_c == NULL condition, described by @sanecz, you can not expect any meaningfull result from that test. It can even cause a crash!

That's because malloc() is not defined to initialize the allocated block of memory in any way. It can contain a zero byte at any position, which will cause strlen() to return any value less than the requested size of the block str_c_len; or it can contain no NUL byte at all, which will cause strlen() to scan past the allocated block and trigger an Undefined Behavior.

Additionally, you do not need the allocated block. If you want only to make an interleaved output, just do that: print characters in appropriate order.

int main(int argc, char const *argv[])
{
    // Expecting: exe name, str_a, str_b
    if (argc != 3)
    {
        return EXIT_FAILURE;
    }

    size_t str_a_len = strlen(argv[1]);
    size_t str_b_len = strlen(argv[2]);
    if (str_a_len != str_b_len)
    {
        return EXIT_FAILURE;
    }

    for (int i = 0; i < str_a_len; ++i)
    {
        putchar( argv[1][i] );
        putchar( argv[2][i] );
    }

    putchar('\n');

    return EXIT_SUCCESS;
}


EDIT

As for the comment 'this was mainly an exercise in string
manipulation': if you want to improve your programming skills, and string
manipulation among them, then I'd recommend to organize your program
into separate functions. For example:

  • let main() fetch input data and pass it to some processing routine,


then print the output;

  • let some MakeInterleavedString() validate input and prepare output


buffer;

  • and let some InterleaveTwoStrings() do actual processing.



For example:

char *InterleaveTwoStrings(const char *l, const char *r, char *sum)
{
    // assume input strings l, r of equal length
    // and output buffer sum twice that long

    char *out = sum;      // the place where output goes
    while(*l != 0)        // input not exhausted yet
    {
        *out ++ = *l ++;  // copy one char and advance input
        *out ++ = *r ++;  // and output positions
    }
    *out = 0;             // terminate the output string

    return sum;           // return the filled buffer
}

char *MakeInterleavedString(const char *l, const char *r)
{
    // test if strings l, r are equal length,
    // prepare a buffer to accomodate l,r contents
    // interleave data into a buffer and return it;
    // return NULL on error

    int llen = strlen(l), rlen = strlen(r);
    if(llen == rlen)
    {
        if(char *sum = malloc(2*llen + 1))    // +1 for terminating NUL char
            return InterleaveTwoStrings(l, r, sum);
    }

    return NULL;   // on (llen != rlen) or malloc() failure
}

int main(int argc, char const *argv[])
{
    // Expecting: exe name, str_a, str_b
    if (argc == 3)
    {
        const char *str_a = argv[1];
        const char *str_b = argv[2];

        if (const char *str_c = MakeInterleavedString(str_a,str_b))
        {
            printf("%s\n", str_c);
            free(str_c);
            return EXIT_SUCCESS;
        }
    }
    return EXIT_FAILURE;
}


Please note the call to free() function – it's not that important in such simple program, as all the allocated memory goes back to the operating system on the program termination. However, in library functions we should always release temporary buffers as soon as they are no longer used. Otherwise we may forget to release them later, and we may cause so called 'memory leaks', when long running process consumes more and more memory for no reason.

Code Snippets

char * str_c = malloc(str_c_len);
if (strlen(str_c) == 0)
    ....
int main(int argc, char const *argv[])
{
    // Expecting: exe name, str_a, str_b
    if (argc != 3)
    {
        return EXIT_FAILURE;
    }

    size_t str_a_len = strlen(argv[1]);
    size_t str_b_len = strlen(argv[2]);
    if (str_a_len != str_b_len)
    {
        return EXIT_FAILURE;
    }

    for (int i = 0; i < str_a_len; ++i)
    {
        putchar( argv[1][i] );
        putchar( argv[2][i] );
    }

    putchar('\n');

    return EXIT_SUCCESS;
}
char *InterleaveTwoStrings(const char *l, const char *r, char *sum)
{
    // assume input strings l, r of equal length
    // and output buffer sum twice that long

    char *out = sum;      // the place where output goes
    while(*l != 0)        // input not exhausted yet
    {
        *out ++ = *l ++;  // copy one char and advance input
        *out ++ = *r ++;  // and output positions
    }
    *out = 0;             // terminate the output string

    return sum;           // return the filled buffer
}

char *MakeInterleavedString(const char *l, const char *r)
{
    // test if strings l, r are equal length,
    // prepare a buffer to accomodate l,r contents
    // interleave data into a buffer and return it;
    // return NULL on error

    int llen = strlen(l), rlen = strlen(r);
    if(llen == rlen)
    {
        if(char *sum = malloc(2*llen + 1))    // +1 for terminating NUL char
            return InterleaveTwoStrings(l, r, sum);
    }

    return NULL;   // on (llen != rlen) or malloc() failure
}

int main(int argc, char const *argv[])
{
    // Expecting: exe name, str_a, str_b
    if (argc == 3)
    {
        const char *str_a = argv[1];
        const char *str_b = argv[2];

        if (const char *str_c = MakeInterleavedString(str_a,str_b))
        {
            printf("%s\n", str_c);
            free(str_c);
            return EXIT_SUCCESS;
        }
    }
    return EXIT_FAILURE;
}

Context

StackExchange Code Review Q#155003, answer score: 7

Revisions (0)

No revisions yet.