Мне действительно нужно использовать malloc здесь? Что-то не так с тем, как я объявляю этот массив?
Это зависит. VLA: s был удален как обязательный компонент из C11, поэтому, строго говоря, вы используете расширения компилятора, тем самым уменьшая переносимость. В будущем VLA может быть удален из вашего компилятора. Может быть, вы также хотите перекомпилировать код на компиляторе без поддержки VLA: s. Анализ рисков по этому вопросу остается за вами.
Другая проблема заключается в том, что при распределении происходит сбой. Если вы используете malloc, у вас есть шанс восстановиться после этого, но если вы только собираетесь сделать что-то вроде этого:
unsigned char *fileData = malloc(itemsToRead);
if(!fileData)
exit(EXIT_FAILURE);
То есть просто выйдите из строя при неудаче и не пытайтесь восстановитьтогда это не очень важно. По крайней мере, не с точки зрения чистого восстановления.
Но также, хотя стандарт C не предъявляет никаких требований к тому, что VLA попадает в стек или кучу, насколько я знаю, это довольно распространено дляположить их в стек. Это означает, что риск неудачного выделения из-за недостатка доступной памяти намного, намного выше. В Linux размер стека обычно составляет 8 МБ, а в Windows - 1 МБ. Почти во всех случаях доступная куча намного выше. Объявление char arr[n] в основном совпадает с char *arr = alloca(n), за исключением того, как работает оператор sizeof.
VLA: s не являются заменой malloc. Они являются заменой для alloca. Если вы не хотите менять malloc на alloca, то вам также не следует переходить на VLA.
Есть много вещей, которые следует учитывать, но я бы не стал использовать VLA. : s. Если вы спросите меня, самый большой риск для них заключается в том, что, поскольку они так просты в использовании, люди становятся небрежными с ними. В тех немногих случаях, когда я нахожу их подходящими, я бы использовал alloca вместо этого, потому что тогда я не скрываю опасности.
Краткое резюме
VLA:S11 и более поздние не требуются, поэтому, строго говоря, вы полагаетесь на расширения компилятора.
VLA: s являются синтаксическим сахаром для alloca, а не malloc. Так что не используйте их вместо malloc. За исключением того, как sizeof работает на VLA, они не дают абсолютно никакой выгоды, за исключением несколько более простого объявления.
VLA: s (обычно) хранятся в стекеВ то время как выделение выполнено, мой malloc (обычно) хранится в куче, поэтому большое выделение имеет гораздо более высокий риск сбоя.
Эта функция отлично работает.
Нет, это не так. У него неопределенное поведение. Как отметил Джонатан Леффлер в комментариях, массив fileName слишком короткий. Требуется не менее 12 байтов для включения \0 -терминатора. Вы можете сделать это немного безопаснее, изменив:
snprintf(fileName,
sizeof(fileName),
"%s_%u%u%u%s",
"LOG", day, date, month, ".bin");
В этом случае проблема со слишком маленьким массивом проявится, создав файл с расширением .bi вместо .bin, которыйЛучшая ошибка, чем неопределенное поведение, которое является текущим случаем.
В вашем коде также нет проверок на ошибки. Я бы переписал это так. А для тех, кто думает, что goto - это плохо, обычно так и есть, но обработка ошибок практична и общепринята среди опытных программистов на Си. Другим распространенным применением является разрыв вложенных циклов, но здесь это неприменимо.
int memory_get_log(unsigned char day, unsigned char date, unsigned char month){
char fileName[12];
unsigned long readItems, itemsToRead;
int ret = 0;
F_FILE *file;
snprintf(fileName,
sizeof(fileName),
"%s_%u%u%u%s", "LOG",
day, date, month, ".bin");
file = f_open(fileName , "r");
if(!file) {
ret = 1;
goto END;
}
itemsToRead = f_filelength( fileName );
unsigned char *fileData = malloc(itemsToRead);
if(!fileData) {
ret=2;
goto CLOSE_FILE;
}
readItems = f_read(fileData, 1, itemsToRead, file);
// Maybe not necessary. I don't know. It's up to you.
if(readItems != itemsToRead) {
ret=3;
goto FREE;
}
// Assuming transmit_data have some kind of error check
if(!transmit_data(fileData, itemsToRead)) {
ret=4;
}
FREE:
free(fileData);
CLOSE_FILE:
f_close(file);
END:
return ret;
}
Если функция возвращает только 0, возвращать что-либо бессмысленно. Объявите это как недействительное вместо этого. Теперь я использовал возвращаемое значение, чтобы вызывающий мог обнаружить ошибки и тип ошибки.