0

I'm a bit new to C here, and just wanted to understand a few things about pointers, pointers to pointers, and strings. Here's what I have written down so far:

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

/* Return a pointer to an array of two strings. The first is the characters
   of string s that are at even indices and the second is the characters from
   s that are at odd indices */

char **parity_strings(const char *s) {
    int len = strlen(s);
    char **lst = malloc(sizeof(char*)*2);
    for(int i=0; i<2; i++){
        lst[i] = malloc(sizeof(char)*(len/2));
    }
    for(int i=0; i<len; i++){
        if(i%2==0){
            (*lst)[0]=s[i];
        }else{
            (*lst)[1]=s[i];
        }
    }
    return lst;

}

int main(int argc, char **argv) {
    char **r = parity_strings(argv[1]);
    printf("%s %s %s", r[0], r[1], argv[1]);
    return 0;
}

So I want to dynamically allocate the memory needed for both the string array, and the strings themselves. I want to know if I have the right idea here by wanting to return a pointer of type char** which points to two pointers of type char*, and then from there access each char of the string.

My output is not as expected. I'm relatively new to C, so there are things I'm still learning about.

Any help would be appreciated, thanks.

4
  • 4
    lst[i] = malloc(sizeof(char)*(len/2)); 1) this will allocate too little if the string length happens to be odd 2) you also need space for the two '\0' terminators in the result strings. 2a) you need to null-terminate the resulting strings 3) (*lst)[0]=s[i]; this is just wrong. What is your intention here? Commented Jan 25, 2017 at 14:15
  • C strings are null terminated arrays and it looks like you forgot to process those nulls... Commented Jan 25, 2017 at 14:19
  • @wildplasser I changed it to *(lst[0])=s[i]. I'm trying to access to the first char array Commented Jan 25, 2017 at 14:26
  • @SergeBallesta How would I do that? Commented Jan 25, 2017 at 14:29

4 Answers 4

2

First, make sure to allocate space for the null byte in the output strings:

    for(int i=0; i<2; i++){
        /* Add an extra space for the \0 */
        lst[i] = malloc(sizeof(char)*(len/2 + 1));
    }

Your main problem was the weird (*lst)[0]=s[i]; part. This has to do with how arrays work in C.

The name of an array is best thought of as a pointer to the zeroth element. So, (*lst) is exactly equivalent to writing lst[0]. so (*lst)[0] was just repeatedly overwriting the first character in the first array with the latest even letter, while (*lst)[1] was just repeatedly overwriting the second letter of the first array. The second array was never modified since it was allocated, and just contained random data.

    for(int i=0; i<len; i++){
        if(i%2==0){
            /* i/2 makes sure every space gets filled,
            remember / means integer division in C */
            lst[0][i/2]=s[i];
        }else{
            lst[1][i/2]=s[i];
        }
    }
    /* Null terminate both strings, to be safe */

    /* terminate one early if the string was odd */
    lst[0][len/2 -len%2] = '\0';

    lst[1][len/2 ] = '\0';

    return lst;

}

This solution is 'quick and dirty' - one of the arrays always gets double terminated, but that's ok since we allocated room for both of them.

Sign up to request clarification or add additional context in comments.

2 Comments

"The name of an array is, in fact, a pointer to the first element" - this is not true. An expression of array type will be converted ("decay") to a pointer to the first element if the expression is not the operand of the sizeof or unary & operators, or if it's a string literal used to initialize a character array in a declaration. Arrays are not pointers, period.
For the purposes of someone learning C, or rather, trying to understand how pointers and arrays are used in algorithms, thinking of an array as a pointer to element zero is helpful. I updated the wording to say 'is best thought of,' but I stand by the sentiment.
1
char **split_odd_even (char *org) {

char **lst;
size_t len, idx;

lst = malloc(2 * sizeof *lst);
len = strlen (org);

lst[0] = malloc ((len+3)/2);
lst[1] = malloc ((len+3)/2);

for (idx =0; org[idx]; idx++){
        lst[idx%2][idx/2] = org[idx];
        }

lst[idx%2][idx/2] = 0;
idx++;
lst[idx%2][idx/2] = 0;

return lst;
}

Comments

1

It seems you mean the following function as it is shown in the demonstrative program below

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

char ** parity_strings( const char *s ) 
{
    size_t len = strlen( s );

    char **lst = malloc( 2 * sizeof( char* ) );

    lst[0] = ( char * )malloc( ( len + 1 ) / 2 + 1 );
    lst[1] = ( char * )malloc( len / 2 + 1 );

    size_t i = 0;

    for ( ; i < len; i++ )
    {
        lst[i % 2][i / 2] = s[i];
    }

    lst[i % 2][i / 2] = '\0';
    ++i;
    lst[i % 2][i / 2] = '\0';

    return lst;
}


int main(void) 
{
    char *s[] = { "A", "AB", "ABC", "ABCD", "ABCDE" };

    for ( size_t i = 0; i < sizeof( s ) / sizeof( *s ); i++ )
    {
        char **p = parity_strings( s[i] );

        printf( "%s %s %s\n", p[0], p[1], s[i] );

        free( p[0] );
        free( p[1] );
        free( p );

    }

    return 0;
}

Its output is

A  A
A B AB
AC B ABC
AC BD ABCD
ACE BD ABCDE

As for your function then you incorrectly calculate the lengths of the new arrays and forgot to append them with the terminating zero. Also you should free all the allocated memory when the strings are not needed any more.

And in these statements

(*lst)[0]=s[i];
(*lst)[1]=s[i];

should be written at least like

(*lst)[i / 2] = s[i];
(*( lst + 1 ))[i / 2] = s[i];

Comments

0

Here is a more idiomatic C way directly using pointers instead of indexes:

char **parity_strings(const char *s) {
    int len = strlen(s);
    char **lst = malloc(sizeof(char*)*2);
    for(i=0; i<2; i++){
        lst[i] = malloc((len+3)/2);  // reserve room for the terminating null
    }
    char *even = lst[0], *odd = lst[1];  // initializes pointers to even and odd parts
    for(;;) {                // tests inside the loop
        *even++ = *s++;      // directly processes s pointer!
        if (*s == '\0') break;
        *odd++ = *s++;       // every second is odd...
        if (*s == '\0') break;
    }
    *even = *odd = '\0';     // terminate the strings
    return lst;
}

This way indeed forget the initial s, but you do no need it again, and what is changed is just a local pointer. But as lst must be returned, the code never changes lst[i] but uses copies

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.