Разделите строку и запишите i-й элемент в буфер с помощью C

Вопрос

Недавно я написал следующее splitv() функция без с помощью strtok. Есть ли способ сделать его короче и эффективнее?

Описание

Функция вычисляет index-th элемент строки str разделен на delim. Элемент записывается в buff и 0 возвращается. При возникновении ошибки индекса любой non 0 значение возвращается. Эта функция была вдохновлена ​​функцией разделения Python: buff = str.split(delim)[index], но я думаю, что это работает слишком много и может быть улучшено.

int splitv(char buff[], char str[], char delim[], int index) {
    // returns 0 if an element is written. Else non 0.
    char *start = str, *end;
    int i = 0, buff_i = 0;
    // check if delim not in str
    if ((end = strstr(str, delim)) == NULL) {
        if (index == 0) strcpy(buff, str);
        return index;
    }
    // delim is in str
    for (i = 0; i < index; i++) {
        start = end + strlen(delim);
        end = strstr(start, delim);
        if (end == NULL) {
            // reached the last element
            if (i == index - 1) {
                end = str + strlen(str);
                break;
            }
            // index error: index >= elements_count
            return -1;
        }
    }
    // copy element to buffer
    while (start != end) {
        buff[buff_i] = *start;
        start++;
        buff_i++;
    }
    buff[buff_i] = '';
    return 0;
}

Заранее спасибо!

2 ответа
2

Прежде чем говорить о повышении эффективности вашей функции, давайте поговорим о безопасности:

  1. Вы никогда не проверяете правильность параметров вашей функции.
  2. Вы не знаете, насколько велик buff является.

Для первого выпуска, если str или delim имеет значение null, передавая их непосредственно в strstr вызовет segfault и приведет к сбою вашей программы.

Вторая проблема хуже: если buff меньше, чем str, ваша функция может в конечном итоге засорять память при попытке скопировать из str к buff.
Это неопределенное поведение, и результаты непредсказуемы.

Теперь несколько советов по эффективности:

  1. Рассчитайте длину строки только один раз.
    поскольку strlen работает, перебирая строку в поисках терминатора ‘ 0’, более эффективно вызвать его только один раз в начале вашей функции и сохранить результат в переменной.
    Это также может позволить вам реализовать простой ярлык для крайних случаев, например, когда str или delimiter пустые строки, или разделитель длиннее str.

  2. Тебе не нужно свое while петля в конце и buff_i переменная.
    Вы можете просто использовать memcpy или strncpy чтобы скопировать элемент в буфер.
    memcpy будет немного более эффективным, поскольку он не проверяет ‘ 0’, а strncpy делает.

  3. Поскольку ваша логика такова, что если разделитель никогда не найден, но запрошенный элемент равен 0, результатом будет «успех», вам не нужно обрабатывать это как отдельный пограничный случай.
    Это означает, что вы можете избавиться от первого if блок и обрабатывать все в одном цикле.
    Вы также можете избавиться от специальных проверок в if цикл и просто сосредоточьтесь на поиске.

Следующий пример кода не включает предлагаемые проверки действительности:

int i;
char *start = str;
char *end = start;
ssize_t delim_len = strlen(delim);
ssize_t copy_len;

for (i = 0; i <= index && end != NULL; i++) {
    end = strstr(start, delim);
    if (end != NULL && i != index) start = end + delim_len;
}

/* element not found */
if (i != index) return index;

if (end == NULL) {
    copy_len = strlen(start);
} else {
    copy_len = (ssize_t)(end - start);
}

memcpy(buff, start, copy_len);
buff[copy_len] = '';

return 0;

    int splitv(char buff[], char str[], char delim[], int index) {
        // returns 0 if an element is written. Else non 0.
    

    Несколько моментов об этом интерфейсе:

    • Не должен delim быть указателем на const char? Я думаю, что функция не должна его изменять. Точно так же, если мы можем избежать изменения входной строки (что является одним из основных преимуществ отказа от использования strtok()), то тоже примите это как указатель на константу.
    • Пожалуйста, примите аргумент, чтобы указать вместимость buff() чтобы мы могли писать код без переполнения буфера!
    • index должно быть без знака, поскольку отрицательные значения здесь не имеют смысла.
    • Мы могли бы сделать возвращаемое значение более полезным, вернув длину совпадения (или отрицательное значение в случае неудачи). Как интерфейс к snprintf(), это очень полезно, если буфер слишком мал. Я бы даже согласился NULL, 0 для аргументов буфера таким же образом.
    • Более традиционно аргументы указателя символа записываются как char *arg чем с синтаксисом неопределенного массива. С точки зрения компилятора, конечно, они идентичны, но соблюдение соглашений облегчает чтение кода другим программистам.

    Посмотрим, как мы будем использовать существующий интерфейс:

    #include <stdio.h>
    int main(void)
    {
        char list[] = "Alice, Bob, Charlie, Don";
        char delim[] = ", ";
        char buff[10];              /* is that enough?? */
    
        if (splitv(buff, list, delim, 2)) {
            fputs("Couldn't split the string.n", stderr);
            return 1;
        }
    
        printf("Found %sn", buff);
    }
    

    Посмотрите, насколько проще новый интерфейс:

    int splitv(char *buff, size_t buff_len,
               const char *str, const char *delim,
               unsigned int index);
    
    
    #include <stdio.h>
    int main(void)
    {
        const char *list = "Alice, Bob, Charlie, Don";
        char buff[10];
    
        int len = splitv(buff, sizeof buff, list, ", ", 2);
        if (len < 0) {
            fputs("Couldn't split the string.n", stderr);
            return 1;
        }
        if ((size_t)len >= sizeof buff) {
            fputs("Substring too long.n", stderr);
            return 1;
        }
    
        printf("Found %sn", buff);
    }
    

    Нам не нужно угадывать, достаточно ли велик наш буфер (и адаптивный код может использовать результат для выделения достаточно большого буфера), и мы можем передавать строковые литералы в качестве аргументов.


    const char *start = str, *end;
    int i = 0, buff_i = 0;
    

    Предпочитаю объявлять по одной переменной в каждой строке и предпочитаю объявлять, где вы также можете инициализировать:

    const char *start = str;
    // check if delim not in str
    const char *end = strstr(str, delim);
    if (!end) {
    
    for (int i = 0;  i < index;  ++i) {
    

    У нас есть два бита кода, которые делают то же самое, когда мы достигаем последнего элемента. Это может помочь переосмыслить, что делает этот код:

    1. Пропустить ноль или более подстрок, оканчивающихся на delim.
    2. Найдите следующее вхождение delim, или конец строки.
    3. Скопируйте подстроку.

    Вот упрощенная реализация, которая соответствует предложенному мной интерфейсу и структурирована в соответствии с этими шагами:

    #include <stdint.h>
    #include <string.h>
    
    int splitv(char *buff, size_t buff_len,
               const char *str, const char *delim,
               unsigned int index)
    {
        /* returns length of the substring if it exists */
        /* returns negative on failure */
    
        /* useful constant */
        const size_t delim_len = strlen(delim);
    
        while (index--) {
            /* advance past next delimiter */
            const char *next = strstr(str, delim);
            if (!next) {
                /* not enough delimiters */
                return -1;
            }
            str = next + delim_len;
        }
    
        /* okay, we've found the right substring; now find its end */
        const char *end = strstr(str, delim);
        if (!end) {
            end = str + strlen(str);
        }
    
        /* decide how much to copy, and write it */
        size_t len = (size_t)(end - str);
        if (buff) {
            size_t write_size = len < buff_len  ? len :  buff_len - 1;
            /* write the output */
            memcpy(buff, str, write_size);
            buff[write_size] = 0;
        }
    
        /* return the substring length (not including null terminator) */
        return (int)len;
    }
    

    Обратите внимание, что я поднял strlen(delim) в константу, поскольку мы знаем, что это не нужно повторно вычислять каждый раз, когда проходит цикл. Я также использовал стандартные memcpy функция вместо ручного цикла для записи в buff.

    • 1

      Помимо создания index без подписи, это должен быть size_t, так как эта функция не будет работать на большинстве (всех?) 64-битных системах с более чем 4Б вхождений

      — фары0123

    • 1

      Да, я так думаю. Фактически, на ранней итерации я делал именно это, и я не уверен, почему я изменил это обратно. И int поскольку возвращаемый тип тоже неверен (вероятно, должен быть size_t, с участием (size_t)-1 используется в качестве дозора ошибок. Могу ли я утверждать, что мой код — это не законченный элемент, а всего лишь шаг вперед?

      — Тоби Спейт

    Добавить комментарий

    Ваш адрес email не будет опубликован. Обязательные поля помечены *