Объединить каталог и путь к файлу - C

Question

Как часть изучения C, я написал следующий код, чтобы объединить имя каталога с именем файла.Например: combine("/home/user", "filename") приведет к /home/user/filename.Ожидается, что эта функция будет работать на всех платформах (по крайней мере, во всех популярных дистрибутивах Linux и Windows 32 и 64-битных версиях).

Вот код.

const char* combine(const char* path1, const char* path2)
{
    if(path1 == NULL && path2 == NULL) {
        return NULL;
    }

    if(path2 == NULL || strlen(path2) == 0) return path1;
    if(path1 == NULL || strlen(path1) == 0) return path2;

    char* directory_separator = "";
#ifdef WIN32
    directory_separator = "\\";
#else 
    directory_separator = "/";
#endif

    char p1[strlen(path1)];                    // (1)
    strcpy(p1, path1);                         // (2) 
    char *last_char = &p1[strlen(path1) - 1];  // (3)

    char *combined = malloc(strlen(path1) + 1 + strlen(path2));
    int append_directory_separator = 0;
    if(strcmp(last_char, directory_separator) != 0) {
        append_directory_separator = 1;
    }
    strcpy(combined, path1);
    if(append_directory_separator)
        strcat(combined, directory_separator);
    strcat(combined, path2);
    return combined;
}

У меня есть следующие вопросы относительно приведенного выше кода.

1. Рассмотрим строки с номерами 1,2,3.Все эти 3 строки предназначены для получения последнего элемента из строки.Похоже, я пишу больше кода для такой маленькой вещи.Как правильно получить последний элемент из строки char*.
2. Чтобы вернуть результат, я выделяю новую строку, используя malloc.Я не уверен, что это правильный способ сделать это.Ожидается ли, что вызывающий абонент освободит результат?Как я могу указать звонящему, что он должен освободить результат?Существует ли менее подверженный ошибкам метод?
3. Как вы оцениваете код (плохой, средний, хороший)?Какие области могут быть улучшены?

Любая помощь будет полезна.

Редактировать

Исправлены все обсуждаемые проблемы и реализованы предложенные изменения.Вот обновленный код.

void combine(char* destination, const char* path1, const char* path2)
{
    if(path1 == NULL && path2 == NULL) {
        strcpy(destination, "");;
    }
    else if(path2 == NULL || strlen(path2) == 0) {
        strcpy(destination, path1);
    }
    else if(path1 == NULL || strlen(path1) == 0) {
        strcpy(destination, path2);
    } 
    else {
        char directory_separator[] = "/";
#ifdef WIN32
        directory_separator[0] = '\\';
#endif
        const char *last_char = path1;
        while(*last_char != '\0')
            last_char++;        
        int append_directory_separator = 0;
        if(strcmp(last_char, directory_separator) != 0) {
            append_directory_separator = 1;
        }
        strcpy(destination, path1);
        if(append_directory_separator)
            strcat(destination, directory_separator);
        strcat(destination, path2);
    }
}

В новой версии вызывающая сторона должна выделить достаточно буфера и отправить в метод combine.Это позволяет избежать использования проблем malloc и free.Вот использование

int main(int argc, char **argv)
{
    const char *d = "/usr/bin";
    const char* f = "filename.txt";
    char result[strlen(d) + strlen(f) + 2];
    combine(result, d, f);
    printf("%s\n", result);
    return 0;
}

Есть предложения по улучшению?

Answer 1

И есть утечка памяти:

const char *one = combine("foo", "file");
const char *two = combine("bar", "");
//...
free(one);   // needed
free(two);   // disaster!

Редактировать: Ваш новый код выглядит лучше. Некоторые незначительные стилистические изменения:

• Двойная точка с запятой ;; в строке 4.
• В строке 6 заменить strlen(path2) == 0 на path2[0] == '\0'' или просто !path2[0].
• Аналогично в строке 9.
• Удалить цикл, определяющий last_char, и использовать const char last_char = path1[strlen(path1) - 1];
• Измените if(append_directory_separator) на if(last_char != directory_separator[0]). И поэтому вам больше не нужна переменная append_directory_separator.
• Ваша функция также возвращает destination, аналогично strcpy(dst, src), который возвращает dst.

Редактировать : И ваш цикл для last_char содержит ошибку : он всегда возвращает конец path1, и вы можете получить двойную косую черту // в вашем ответе. (Но Unix будет рассматривать это как одну косую черту, если только это не в начале). Во всяком случае, мое предложение исправляет это - что, я вижу, очень похоже на ответ jdmichal. И Я вижу, что у вас было это правильно в вашем оригинальном коде (на который, я признаю, я только взглянул - это было слишком сложно на мой вкус; ваш новый код намного лучше).

И еще два, несколько более субъективных мнения:

• Я бы использовал stpcpy(), чтобы избежать неэффективности strcat(). (Легко написать свой, если нужно.)
• У некоторых людей очень сильное мнение о strcat() и т. П. Как о небезопасных . Тем не менее, я думаю, что вы используете здесь совершенно нормально.

Answer 2

1. Единственный раз, когда вы используете last_char, это сравнение, чтобы проверить, является ли последний символ разделителем.

Почему бы не заменить его следующим:

/* Retrieve the last character, and compare it to the directory separator character. */
char directory_separator = '\\';
if (path1[strlen(path1) - 1] == directory_separator)
{
    append_directory_separator = 1;
}

Если вы хотите учесть возможность использования нескольких символьных разделителей, вы можете использовать следующее. Но убедитесь, что при выделении объединенной строки добавьте strlen (directory_separator) вместо 1.

/* First part is retrieving the address of the character which is
   strlen(directory_separator) characters back from the end of the path1 string.
   This can then be directly compared with the directory_separator string. */
char* directory_separator = "\\";
if (strcmp(&(path1[strlen(path1) - strlen(directory_separator)]), directory_separator))
{
    append_directory_separator = 1;
}

2. Менее подверженный ошибкам метод заключается в том, чтобы пользователь дал вам буфер назначения и его длину, во многом так же, как работает strcpy. Это дает понять, что они должны управлять распределением и освобождением памяти.

3. Процесс кажется достаточно приличным. Я думаю, что есть только некоторые особенности, над которыми можно поработать, в основном, неуклюже. Но у вас все хорошо, вы уже можете распознать это и попросить о помощи.

Answer 3

Вот что я использую:

#if defined(WIN32)
#  define DIR_SEPARATOR '\\'
#else
#  define DIR_SEPARATOR '/'
#endif

void combine(char *destination, const char *path1, const char *path2) {
  if (path1 && *path1) {
    auto len = strlen(path1);
    strcpy(destination, path1);

    if (destination[len - 1] == DIR_SEPARATOR) {
      if (path2 && *path2) {
        strcpy(destination + len, (*path2 == DIR_SEPARATOR) ? (path2 + 1) : path2);
      }
    }
    else {
      if (path2 && *path2) {
        if (*path2 == DIR_SEPARATOR)
          strcpy(destination + len, path2);
        else {
          destination[len] = DIR_SEPARATOR;
          strcpy(destination + len + 1, path2);
        }
      }
    }
  }
  else if (path2 && *path2)
    strcpy(destination, path2);
  else
    destination[0] = '\0';
}

Answer 4

Просто небольшое замечание для улучшения вашей функции:

Windows поддерживает как '/', так и '\\' разделители в путях. Поэтому я должен быть в состоянии выполнить следующий вызов:

const char* path1 = "C:\\foo/bar";
const char* path2 = "here/is\\my/file.txt";
char destination [ MAX_PATH ];
combine ( destination, path1, path2 );

Идея при написании многоплатформенного проекта может заключаться в преобразовании '\\' в '/' в любом пути ввода (из пользовательского ввода, загруженных файлов ...), тогда вам придется иметь дело только с '/' символами.

Привет.

Answer 5

Быстрый взгляд показывает:

1. вы используете комментарии C ++ (//), которые не являются стандартными C
2. вы объявляете переменные частично вниз по коду - тоже не C. Они должны быть определены в начале функции.
3. ваша строка p1 в # 1 имеет слишком много байтов, записанных в # 2, потому что strlen возвращает длину строки, и вам нужно еще 1 байт для нулевого терминатора.
4. malloc не выделяет достаточно памяти - вам нужна длина path1 + длина path2 + длина разделителя + нулевой терминатор.