Почему моя реализация string_split не работает? - PullRequest
2 голосов
/ 26 марта 2019

Моя str_split функция возвращает (или, по крайней мере, я так думаю) char** - так что, по сути, список строк.Требуется строковый параметр, разделитель char для разделения строки и указатель на int для размещения количества обнаруженных строк.

Способ, которым я это сделал, что может быть крайне неэффективным, должен создать буфер x длины (x = длина строки), затем скопировать элемент строки, пока мы не достигнем разделителя, или '\0' символа.Затем он копирует буфер в char**, который мы и возвращаем (и был malloc отредактирован ранее, и может быть освобожден из main()), затем очищает буфер и повторяется.

Хотя алгоритм может быть ненадежным, логика определенно правильная, поскольку мой отладочный код (_D) показывает, что он копируется правильно.Часть, на которой я застрял, это когда я делаю char** в main, устанавливаю его равным моей функции.Он не возвращает ноль, не вызывает сбой программы и не выдает никаких ошибок, но, похоже, тоже не работает.Я предполагаю, что это то, что подразумевается под термином «неопределенное поведение».

Во всяком случае, после долгих размышлений (я новичок во всем этом) я попробовал что-то еще, что вы увидите в коде, в настоящее время закомментированном.Когда я использую malloc, чтобы скопировать буфер в новую строку и передать эту копию вышеупомянутому символу **, он, кажется, работает отлично.ОДНАКО, это создает очевидную утечку памяти, так как я не могу освободить ее позже ... так что я потерялся.

Когда я провел какое-то исследование, я обнаружил этот пост , который почти точно соответствует идее моего кода и работает, то есть нет никакой внутренней проблемы с форматом (возвращаемое значение, параметрыи т.д.) моей функции str_split.Но у него только 1 malloc для символа **, и он работает просто отлично.

Ниже приведен мой код.Я пытался понять это, и это затягивает мой мозг, поэтому я очень ценю помощь !!Заранее извините за 'i', 'b', 'c', это немного запутанно, я знаю.

Edit : следует упомянуть об этом с помощью следующего кода,

ret[c] = buffer;
printf("Content of ret[%i] = \"%s\" \n", c, ret[c]);

это действительно печатает правильно.Только когда я вызываю функцию из main, она становится странной.Я предполагаю, что это потому, что это выходит за рамки?

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

#define DEBUG

#ifdef DEBUG
    #define _D if (1)
#else
    #define _D if (0)
#endif

char **str_split(char[], char, int*);
int count_char(char[], char);

int main(void) {
    int num_strings = 0;
    char **result = str_split("Helo_World_poopy_pants", '_', &num_strings);

    if (result == NULL) {
        printf("result is NULL\n");
        return 0;
    }

    if (num_strings > 0) {
        for (int i = 0; i < num_strings; i++) {
            printf("\"%s\" \n", result[i]);
        }
    }

    free(result);

    return 0;
}

char **str_split(char string[], char delim, int *num_strings) {

    int num_delim = count_char(string, delim);
    *num_strings = num_delim + 1;

    if (*num_strings < 2) {
        return NULL;
    }

    //return value
    char **ret = malloc((*num_strings) * sizeof(char*));

    if (ret == NULL) {
        _D printf("ret is null.\n");
        return NULL;
    }

    int slen = strlen(string);
    char buffer[slen];

    /* b is the buffer index, c is the index for **ret */
    int b = 0, c = 0;
    for (int i = 0; i < slen + 1; i++) { 

        char cur = string[i];

        if (cur == delim || cur == '\0') {

            _D printf("Copying content of buffer to ret[%i]\n", c); 
            //char *tmp = malloc(sizeof(char) * slen  + 1);
            //strcpy(tmp, buffer);

            //ret[c] = tmp;
            ret[c] = buffer;
            _D printf("Content of ret[%i] = \"%s\" \n", c, ret[c]);
            //free(tmp);

            c++;
            b = 0;
            continue;
        }

        //otherwise

        _D printf("{%i} Copying char[%c] to index [%i] of buffer\n", c, cur, b);

        buffer[b] = cur;
        buffer[b+1] = '\0'; /* extend the null char */
        b++;

        _D printf("Buffer is now equal to: \"%s\"\n", buffer);
    }

    return ret;
}

int count_char(char base[], char c) {
    int count = 0;
    int i = 0;

    while (base[i] != '\0') {
        if (base[i++] == c) {
            count++;
        }
    }
    _D printf("Found %i occurence(s) of '%c'\n", count, c);
    return count;
}

Ответы [ 2 ]

0 голосов
/ 26 марта 2019

Вы храните указатели на буфер, который существует в стеке. Использование этих указателей после возврата из функции приводит к неопределенному поведению.

Чтобы обойти это, требуется одно из следующих действий:

  • Разрешить функции изменять входную строку (т.е. заменять разделители символами нулевого конца) и возвращать в нее указатели. Звонящий должен знать, что это может произойти. Обратите внимание, что предоставление строкового литерала, как вы делаете здесь, является недопустимым в C, поэтому вместо этого вам нужно будет сделать:

    char my_string[] = "Helo_World_poopy_pants";
    char **result = str_split(my_string, '_', &num_strings);
    

    В этом случае функция также должна прояснить, что строковый литерал является недопустимым вводом, и определить свой первый параметр как const char* string (вместо char string[]).

  • Разрешить функции сделать копию строки, а затем изменить копию. Вы выразили обеспокоенность по поводу утечки этой памяти, но в основном это связано с дизайном вашей программы, а не с необходимостью.

    Совершенно верно дублировать каждую строку по отдельности, а затем очищать их все позже. Основная проблема в том, что это неудобно, а также немного бессмысленно.

Давайте обратимся ко второму пункту. У вас есть несколько вариантов, но если вы настаиваете, чтобы результат был легко очищен с помощью вызова free, попробуйте следующую стратегию:

  1. Когда вы выделяете массив указателей, также сделайте его достаточно большим, чтобы вместить копию строки:

    // Allocate storage for `num_strings` pointers, plus a copy of the original string,
    // then copy the string into memory immediately following the pointer storage.
    char **ret = malloc((*num_strings) * sizeof(char*) + strlen(string) + 1);
    char *buffer = (char*)&ret[*num_strings];
    strcpy(buffer, string);
    
  2. Теперь выполните все ваши строковые операции на buffer. Например:

    // Extract all delimited substrings.  Here, buffer will always point at the
    // current substring, and p will search for the delimiter.  Once found,
    // the substring is terminated, its pointer appended to the substring array,
    // and then buffer is pointed at the next substring, if any.
    int c = 0;
    for(char *p = buffer; *buffer; ++p)
    {
        if (*p == delim || !*p) {
           char *next = p;
           if (*p) {
               *p = '\0';
               ++next;
           }
           ret[c++] = buffer;
           buffer = next;
        }
    }
    
  3. Когда вам нужно очистить, это всего лишь один вызов free, потому что все было сохранено вместе.

0 голосов
/ 26 марта 2019

Строковые указатели, которые вы храните в массиве res с ret[c] = buffer;, указывают на автоматический массив, который выходит из области видимости, когда функция возвращается.Код впоследствии имеет неопределенное поведение.Вы должны распределить эти строки с помощью strdup().

. Обратите также внимание, что может быть нецелесообразно возвращать NULL, если строка не содержит разделителя.Почему бы не вернуть массив с одной строкой?

Вот более простая реализация:

#include <stdlib.h>

char **str_split(const char *string, char delim, int *num_strings) {
    int i, n, from, to;
    char **res;

    for (n = 1, i = 0; string[i]; i++)
        n += (string[i] == delim);

    *num_strings = 0;
    res = malloc(sizeof(*res) * n);
    if (res == NULL)
        return NULL;

    for (i = from = to = 0;; from = to + 1) {
        for (to = from; string[to] != delim && string[to] != '\0'; to++)
            continue;
        res[i] = malloc(to - from + 1);
        if (res[i] == NULL) {
            /* allocation failure: free memory allocated so far */
            while (i > 0)
                free(res[--i]);
            free(res);
            return NULL;
        }
        memcpy(res[i], string + from, to - from);
        res[i][to - from] = '\0';
        i++;
        if (string[to] == '\0')
            break;
    }
    *num_strings = n;
    return res;
}
...