Мне действительно нужно использовать 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, возвращать что-либо бессмысленно. Объявите это как недействительное вместо этого. Теперь я использовал возвращаемое значение, чтобы вызывающий мог обнаружить ошибки и тип ошибки.