0

Working on a program to split strings into smaller strings with trailing whitespace removed, in an attempt to fully get knowledge of pointers in my head. However, when I try and do pointer arithmetic on a array of pointers, I keep getting a segmentation fault (tested and worked out this was the specific line where the fault occurs); Here's my code:

int enfold(char* to_enfold, long unsigned line_length, char** enfolded) {
    int k;

    long unsigned char_num = strlen(to_enfold);
    if (char_num < line_length) {
        for (k = 0; k <= sizeof(to_enfold); k++)
            enfolded[0][k] = to_enfold[k];

        printf("TOO SHORT\n");

        enfolded[0][k] = '\n';

        return 1;
        }

    else {
        int i = LINE_LENGTH-1;

        while ( to_enfold[i] == ' ' ||
                to_enfold[i] == '\t' ||
                to_enfold[i] == '\n' ||
                i == 0) {
            printf("%d\n", i);
            i--;
        }

        for (k = 0; k <= i; k++)
            enfolded[0][k] = to_enfold[k];

        enfolded[0][k] = '\n';

        return 1 + enfold((to_enfold+LINE_LENGTH), line_length, (enfolded+1));
    }
}

The problem is with the recursion, which causes a segmentation fault for using arithmetic for (enfolded+1), though not if we overwrite using enfolded. Is there a problem with using pointer arithmetic on pointers to pointers.

1
  • You are passing in a two-dimensional char * array, implying the function will create multiple strings, but then you only access element [0]. If that is the intent you don't need an array and can pass in a char *. If this is just a first cut, maybe try get it working with only a char * first. Minor nit - the first parameter should be "const char *" to reflect it will not be updated. Commented Dec 21, 2013 at 3:22

1 Answer 1

1

One problem is the use of sizeof() in the code:

if (char_num < line_length) {
    for (k = 0; k <= sizeof(to_enfold); k++)
        enfolded[0][k] = to_enfold[k];

That should be strlen(), except you don't call strlen() in the condition of a loop, and in any case the loop should be written:

    strcpy(enfolded[0], to_enfold);

However, that isn't the part of the code where you are seeing the problem. Since you've not shown us how the memory is allocated for enfolded, it is hard to know what you've done, but there's a very good chance that you've not allocated the memory correctly. You should have pre-allocated not only the array of pointers, but also the array that each of the pointers points at (since you don't allocate the space in this code). Alternatively, you need to allocate the requisite space here. You also have not told this function how many pointers are in the array. Fiendishly crafted input can therefore easily cause you to write out of bounds. You need to know how much space there is in the array of pointers.

So, I think you should:

  1. Allocate the strings in this function.
  2. Make sure you free them all when you're done.
  3. Make sure you know how many strings can be stored in the array.

If you're on a system with valgrind then use that to guide you through memory allocation and release.


Please read up on how to create an SSCCE (Short, Self-Contained, Correct Example). One reason why people may not have jumped in to help is that your code self-evidently is not an SSCCE; there is no main() function. Here is an approximation to an SSCCE that works. The main change I made was to ensure that the strings are null terminated; I'm fairly sure that was a large part of your problem.

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

static
int enfold(char *to_enfold, size_t line_length, char **enfolded)
{
    size_t k;

    size_t char_num = strlen(to_enfold);
    if (char_num < line_length)
    {
        for (k = 0; k <= char_num; k++)
            enfolded[0][k] = to_enfold[k];
        printf("TOO SHORT\n");
        enfolded[0][k] = '\n';
        enfolded[0][k+1] = '\0';
        return 1;
    }
    else
    {
        size_t i = line_length - 1;

        while (to_enfold[i] == ' ' ||
               to_enfold[i] == '\t' ||
               to_enfold[i] == '\n' ||
               i == 0)
        {
            printf("%zu\n", i);
            i--;
        }

        for (k = 0; k <= i; k++)
            enfolded[0][k] = to_enfold[k];

        enfolded[0][k] = '\n';
        enfolded[0][k+1] = '\0';

        return 1 + enfold((to_enfold + line_length), line_length, (enfolded + 1));
    }
}

int main(void)
{
    enum { SIZE = 100 };
    char *enfolded[SIZE];

    for (int i = 0; i < SIZE; i++)
        enfolded[i] = malloc(sizeof(char) * SIZE);

    char line[4096];
    while (fgets(line, sizeof(line), stdin) != 0)
    {
        int n = enfold(line, 8, enfolded);
        assert(n < SIZE);

        for (int i = 0; i < n; i++)
            printf("%d: <<%s>>\n", i, enfolded[i]);
        printf("Parts: %d\n", n);
    }

    for (int i = 0; i < SIZE; i++)
        free(enfolded[i]);

    return 0;
}

Sample run:

$ ./mem <<< "Hello, how are you today?"
TOO SHORT
0: <<Hello, h
>>
1: <<ow are y
>>
2: <<ou today
>>
3: <<?
>>
Parts: 4
$

Checking with valgrind gives the code a clean bill of health. However, you might need to think about what happens if you have 30 blanks in the middle of the string (my strong suspicion is that you'll run into problems, but I've not proven that).

Compilation with GCC 4.8.2 on Mac OS X 10.9.1 Mavericks:

gcc -O3 -g -std=c11 -Wall -Wextra -Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition -Werror mem.c -o mem

I pretty much don't compile with less stringent compilation options than that (unless it needs to be C99 or, perish the thought, C89 code). Note that the code does use some C99 features, notably declarations of variables in the for loop.

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

1 Comment

I allocate the memory on the stack with char* enfolded[SIZE], then use a for loop malloc enfolded[i] = malloc(sizeof(char)*100)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.