#GNSS #CSV #NMEA #URL
В программировании микроконтроллеров часто надо производить синтаксический разбор (парсинг) CSV строчек. CSV это просто последовательность символов, которые разделены запятой (или любым другим одиночным символом: ; | /).
1--CSV строчки можно, например, повстречать в NMEA протоколе от навигационных GNSS приемников. Вот пример NMEA протокола
$GNGGA,102030.000,5546.95900,N,03740.69200,E,1,08,2.0,142.0,M,0.0,M,,*
$GNGLL,5546.95900,N,03740.69200,E,102030.000,A,A*
$GNGSA,A,3,10,16,18,20,26,27,,,,,,,4.8,2.0,4.3,1*
$GNGSA,A,3,19, , , , , ,,,,,,,4.8,2.0,4.3,4*
$GNGSA,A,3,82, , , , , ,,,,,,,4.8,2.0,4.3,2*
$GPGSV,3,1,12,07,08,343,,08,07,304,,10,28,195,42,13,20,054,,0*
$GPGSV,3,2,12,15,27,087,,16,47,262,39,18,66,082,23,20,58,174,23,0*
$GPGSV,3,3,12,21,75,089,23,26,33,222,31,27,38,298,40,29,15,127,,0*
$BDGSV,1,1,01,19,29,174,28,0*
$GLGSV,3,1,09,74,08,001,34,66,55,096,,82,69,318,21,73,25,326,,0*
$GLGSV,3,2,09,80,20,258,,65,18,025,,83,21,292,,81,51,092,,0*
$GLGSV,3,3,09,67,26,161,,0*
$GNRMC,102030.000,A,5546.95900,N,03740.69200,E,0.12,49.75,200220,,,A,V*
$GNVTG,49.75,T,,M,0.12,N,0.22,K,A*
$GNZDA,102030.000,20,02,2020,00,00*
$GPTXT,01,01,01,ANTENNA OK*
$GNDHV,102030.000,0.03,0.000,0.000,0.000,0.00,,,,,M*
$GNGST,102030.000,6.9,,,,5.6,9.2,10.1*
$GPTXT,01,01,02,MS=7,7,061A8200,33,0,00000000,20,2,00028000*Поэтому можно сказать, что CSV это космический протокол для приема сигналов со спутников!
2-- Потом, любой URL (например этот https://habr.com/ru/articles/765066/) это, в сущности, та же самая пресловутая CSV строчка, где разделитель это /.
3-- Также компонент CSV позволит Вам одной строчкой в UART-CLI консоли прошивки распознавать и запускать на исполнение последовательно сразу несколько shell команд.
4--Прошивка может запросто логировать на SD карту по SPI данные в CSV формате как в файл на FatFS. Потом этот текстовый *.csv файлик можно будет открыть на LapTop(е) любым процессором электронных таблиц.
Постановка задачи
Дана строка, например "aaa,bbbb,cccc,,eeeee,fffff". Также дан индекс элемента в виде положительного целого числа. Например число два. Вернуть подстроку, которая соответствует индексу. Отсчет начитать от нуля. В данном случае результат это "сссс".
Вот несколько тестовых случаев. В данном случае в качестве разделителя служит запятая.
# теста |
Строка |
индекс |
результат |
количество элементов |
1 |
"aaa,bbbb,cccc,,eeeee,fffff" |
5 |
"fffff" |
6 |
2 |
"aaa,bbbb,cccc,,eeeee,fffff" |
0 |
"aaa" |
6 |
3 |
"aaa,bbbb,cccc,,eeeee,fffff" |
1 |
"bbbb" |
6 |
4 |
"aaa,bbbb,cccc,,eeeee,fffff" |
3 |
"" |
6 |
5 |
"," |
0 |
"" |
2 |
6 |
"aaa,bbb,ccc,,eee,fff" |
6 |
Error |
6 |
При этом API должен быть именно такой
bool csv_parse_text(const char* const text,
char separator,
uint32_t index,
char* const sub_str,
uint32_t sub_str_size);где
# аргумента |
имя |
тип данных |
Пояснение |
1 |
text |
char* |
Входная CSV строка |
2 |
separator |
char |
символ разделителя |
3 |
index |
uint32_t |
индекс колонки которую надо извлечь |
4 |
sub_str |
char* |
извлеченная подстрока |
5 |
sub_str_size |
uint32_t |
размер выходного массива |
В случае какой-либо ошибки (например выход за пределы предоставленной для sub_str памяти или неверном индексе) вернуть false. При этом функция csv_parse_text() не должна менять исходную строку text. Строка text вообще может лежать в NOR-Flash памяти. И крайне желательно написать код в соответствии со стандартом ISO26262-6. Вот, пожалуй, и все требования к программному компоненту CSV decoder.
Для человека тут всё очевидно. Однако как заставить компьютерную программу выделять подстроки из CSV строчек?
Как же извлекать текст из CSV строчек?
Можно сказать, что СSV это своеобразный текстовый протокол для упаковки переменных в пакет. Как это обычно и бывает в программировании все задачи решаются золотым шаблоном: конечным автоматом. Надо спроектировать конечный автомат.
Фаза 1. Определить входы конечного автомата
№ входа |
Пояснение |
Токен |
1 |
Символ |
NOT_SEP |
2 |
Разделитель |
Sep |
3 |
Конец строки |
End |
Фаза 2. Определить состояния конечного автомата.
№ состояния |
Пояснение |
Токен |
1 |
Конечный автомат только проинициализирован |
INIT |
2 |
Накапливаем данные в ячейку |
ACCUMULATE |
3 |
обнаружен разделитель |
SEP |
4 |
Работа выполнена |
END |
Фаза 3. Нарисовать граф переходов между состояниями
Взаимосвязь между состояниями и входными воздействиями показывает граф конечного автомата. Это простой планарный граф.
Теперь у нас есть всё чтобы начать писать код.
Фаза 4. Написать программный код для парсера CSV строчек
У каждого программного компонента есть свои константы. Вот они для компонента CSV.
#ifndef CSV_CONST_H
#define CSV_CONST_H
#ifndef HAS_CSV
#error "+HAS_CSV"
#endif
typedef enum {
CSV_STATE_INIT = 1,
CSV_STATE_ACCUMULATE = 2,
CSV_STATE_SEP = 3,
CSV_STATE_END = 4,
CSV_STATE_UNDEF = 0,
} CsvState_t;
typedef enum {
CSV_INPUT_NOT_SEP = 1,
CSV_INPUT_SEP = 2,
CSV_INPUT_END = 3,
CSV_INPUT_UNDEF = 0,
} CsvInput_t;
#endif /*CSV_CONST_H*/
У каждого программного компонента есть свои специфические типы данных.
#ifndef CSV_TYPEES_H
#define CSV_TYPEES_H
#ifdef __cplusplus
extern "C" {
#endif
#include <stdbool.h>
#include <stdint.h>
#ifndef HAS_CSV
#error "+HAS_CSV"
#endif
#include "csv_const.h"
typedef struct {
char* out_buff; /*result array*/
char prev_char;
char separator;
bool init_done;
bool fetch_done;
char symbol;
uint32_t out_size;
uint32_t i;/*char index*/
int32_t fetch_index; /*starts from 0; -1 if fetch is not needed*/
uint32_t cnt; /*total number of cells*/
uint32_t position; /*starts from 0*/
CsvState_t state;
CsvInput_t input;
} CsvFsm_t;
#ifdef __cplusplus
}
#endif
#endif /* CSV_TYPEES_H */
А это API программного компонента CSV. Тут основная функция это csv_parse_text()
#ifndef CSV_H
#define CSV_H
#ifdef __cplusplus
extern "C" {
#endif
#include <stdbool.h>
#include <stdint.h>
#include "csv_types.h"
uint32_t csv_cnt(const char* const text,
char separator);
bool csv_parse_text(const char* const in_text,
char separator,
uint32_t index,
char* const text,
uint32_t size);
#ifdef __cplusplus
}
#endif
#endif /* CSV_H */
Инициализация конечного автомата разбора CSV
static bool csv_init(CsvFsm_t* Node,
char separator,
int32_t fetch_index, char* const out_text, uint32_t size) {
bool res = false;
LOG_DEBUG(CSV, "Init: Separator:%c", separator);
if(Node) {
Node->separator = separator;
Node->init_done = true;
Node->fetch_done = false;
Node->prev_char = 0x00;
Node->state = CSV_STATE_INIT;
Node->input = CSV_INPUT_UNDEF;
Node->cnt = 0;
Node->position = 0;
Node->i = 0;
Node->out_buff = out_text;
Node->out_size = size;
Node->fetch_index = fetch_index;
LOG_DEBUG(CSV, "ValMaxSize:%u byte FetchIndex: %d",size, fetch_index);
res = true;
}
return res;
}
uint32_t csv_cnt(const char* const text, char separator) {
bool res = false;
size_t len = strlen(text);
LOG_DEBUG(CSV, "Text:[%s] Size:%u byte Separator:%c", text, len, separator);
CsvFsm_t Node;
csv_init(&Node, separator, -1, NULL, 0);
uint32_t i = 0;
for(i = 0; i < len; i++) {
Node.input = CSV_INPUT_UNDEF;
res = csv_cnt_proc(&Node, text[i]);
}
Node.input = CSV_INPUT_END;
res = csv_cnt_proc(&Node, 0x00);
return Node.cnt;
}
Всю работу делает функция csv_parse_text()
bool csv_parse_text(const char* const in_text, char separator, uint32_t index,
char* const out_text, uint32_t size) {
bool res = false;
if(in_text && out_text) {
size_t len = strlen(in_text);
LOG_DEBUG(CSV, "InText:[%s] Size:%u Sep:%c Index:%u", in_text, len, separator, index);
CsvFsm_t Item={0};
csv_init(&Item, separator, index, out_text, size);
uint32_t i = 0;
for(i = 0; i < len; i++) {
Item.input = CSV_INPUT_UNDEF;
res = csv_cnt_proc(&Item, in_text[i]);
}
Item.input = CSV_INPUT_END;
res = csv_cnt_proc(&Item, 0x00);
res = false;
if(Item.fetch_done ) {
if(0==Item.error_cnt){
res = true;
}
}
}
return res;
}
Собственно, сами шестерни конечного автомата прокручивает функция csv_cnt_proc()
static CsvInput_t csv_symbol_2_input(CsvFsm_t* CsvFsm,
char symbol) {
if(symbol == CsvFsm->separator) {
CsvFsm->input = CSV_INPUT_SEP;
} else {
CsvFsm->input = CSV_INPUT_NOT_SEP;
}
return CsvFsm->input;
}
static bool csv_add_letter(CsvFsm_t* const Node, uint32_t index, char letter, bool last_char){
bool res = true;
if(Node->position==Node->fetch_index){
//Node->i = index;
if(index<Node->out_size){
Node->out_buff[index] = letter ;//Node->symbol;
res = true;
if(last_char){
Node->fetch_done = last_char;
LOG_PROTECTED(CSV, "CSV[%u]=[%s]",Node->position, Node->out_buff);
}
}else{
Node->error_cnt++;
res = false;
}
}
return res;
}
static bool csv_cnt_proc(CsvFsm_t* Node, char symbol) {
bool res = false;
if(Node) {
Node->symbol = symbol;
if(CSV_INPUT_UNDEF == Node->input) {
csv_symbol_2_input(Node, symbol);
}
CsvNodeDiag(Node);
switch(Node->state) {
case CSV_STATE_INIT:
res = csv_cnt_init_proc(Node);
break;
case CSV_STATE_ACCUMULATE:
res = csv_cnt_acc_proc(Node);
break;
case CSV_STATE_SEP:
res = csv_cnt_sep_proc(Node);
break;
case CSV_STATE_END:
res = csv_cnt_end_proc(Node);
break;
default:
break;
}
Node->prev_char = symbol;
}
return res;
}
Обработчик входов для состояния сразу после инициализации
static bool csv_cnt_init_proc(CsvFsm_t* Node) {
bool res = false;
//LOG_DEBUG(CSV, "ProcInit %c Input:%s", Node->symbol, CsvInput2Str(Node->input));
if(Node) {
switch(Node->input) {
case CSV_INPUT_NOT_SEP: {
Node->cnt++;
Node->i = 0;
res=csv_add_letter(Node, 0, Node->symbol, false);
Node->state = CSV_STATE_ACCUMULATE;
} break;
case CSV_INPUT_SEP: {
Node->i = 0;
res=csv_add_letter(Node, 0, 0, true);
Node->cnt++;
Node->position++;
Node->state = CSV_STATE_SEP;
} break;
case CSV_INPUT_END: {
Node->state = CSV_STATE_END;
Node->i = 0;
res=csv_add_letter(Node, 0, 0, true);
Node->cnt++;
} break;
default:
break;
}
}
return res;
}
Обработчик состояния аккумулятора
static bool csv_cnt_acc_proc(CsvFsm_t* Node) {
bool res = false;
//LOG_DEBUG(CSV, "ProcAcc %c Input:%s", Node->symbol, CsvInput2Str(Node->input));
if(Node) {
switch(Node->input) {
case CSV_INPUT_NOT_SEP: {
/*SaveChar*/
Node->state = CSV_STATE_ACCUMULATE;
Node->i++;
res = csv_add_letter(Node, Node->i,Node->symbol, false);
} break;
case CSV_INPUT_SEP: {
Node->i++;
res = csv_add_letter(Node, Node->i,0, true);
Node->i = 0;
Node->position++;
Node->state = CSV_STATE_SEP;
} break;
case CSV_INPUT_END: {
Node->i++;
res = csv_add_letter(Node, Node->i, 0x00, true);
Node->state = CSV_STATE_END;
} break;
default:
res = false;
break;
}
}
return res;
}
обработчик из состояния, когда уже был принят какой-либо разделитель
static bool csv_cnt_sep_proc(CsvFsm_t* Node) {
bool res = false;
//LOG_DEBUG(CSV, "ProcSep %c Input:%s", Node->symbol, CsvInput2Str(Node->input));
if(Node) {
switch(Node->input) {
case CSV_INPUT_NOT_SEP: {
Node->cnt++;
res=csv_add_letter(Node, 0,Node->symbol, false);
Node->state = CSV_STATE_ACCUMULATE;
} break;
case CSV_INPUT_SEP: {
res=csv_add_letter(Node, 0,0, true);
Node->position++;
Node->state = CSV_STATE_SEP;
Node->cnt++;
} break;
case CSV_INPUT_END: {
res=csv_add_letter(Node, 0,0, false);
Node->state = CSV_STATE_END;
Node->cnt++;
} break;
default:
break;
}
}
return res;
}
и обработчик для состояния завершения обработки строки
static bool csv_cnt_end_proc(CsvFsm_t* Node) {
bool res = false;
LOG_DEBUG(CSV, "ProcEnd %c Input:%s", Node->symbol, CsvInput2Str(Node->input));
if(Node) {
switch(Node->input) {
case CSV_INPUT_NOT_SEP: {
res = false;
Node->i = 0;
Node->out_buff[0] = 0x00;
Node->state = CSV_STATE_END;
} break;
case CSV_INPUT_SEP: {
res = false;
Node->i = 0;
Node->out_buff[0] = 0x00;
Node->state = CSV_STATE_END;
} break;
case CSV_INPUT_END: {
res = false;
Node->i = 0;
Node->out_buff[0] = 0x00;
Node->state = CSV_STATE_END;
} break;
default:
break;
}
}
return res;
}
Фаза 5. Тестирование
Каждый программный компонент надо тестировать. Вот набор тестовых случаев, которые должны отрабатывать.
CSV строчка |
индекс |
ожидаемый результат |
"3975, 1.667, 27.50, 21:20:36, 7/8/2023, 1520045092" |
5 |
1520045092 |
"4452,0.000,17.00, 00:00:18, 14/7/2023, 1517894674" |
1 |
0.000 |
"ll wm8731 debug;ll i2c debug;tsr 127" |
0 |
ll wm8731 debug |
"" |
0 |
"" |
"aa;bb;cc" |
3 |
Ошибка |
Сборка и прогон этого кода через модульные тесты на PC показали, что всё работает!
Достоинства CVS протокола и данного парсера в частности
1--Простота извлечения данных по индексу.
2--Человеко-читаемость CVS строчек.
3--Совместим с программами обработки электронных таблиц. Текстовый CSV файл можно загрузить в Google SpreadSheets или Excel.
4--Благодаря этому конечному автомату, СSV строки можно разбирать в потоковом режиме.
5--В представленном решении отсутствует нужда в динамическом выделении памяти, что особенно важно в программировании микроконтроллеров.
6--В каждой функции только 1 return. Это как раз соответствует стандарту ISO26262. Значит этот код можно использовать в автомобильной промышленности.
Недостатки CSV
2--В передаваемом тексте не должно быть самого символа разделителя как данных. Иначе конечный автомат заклинит.
Вывод
Как видите, написать надежный парсер CSV строчек не такая уж и тривиальная задача. Надо спроектировать конечный автомат на 4 состояния, подготовить достаточное количество тестов.
При этом даже наличие текста программы на С не всегда достаточно для понимания работы кода. Мораль этого теста в том, что при решении любой задачи в программировании нельзя сразу кидаться писать код. Надо сперва нарисовать схему конечного автомата.
Исходник это не код, исходник это документация.
Вот Вы и научились выделять подстроки из строк. Далее можно применять распознавание числа из строки. Про это у меня есть отдельный текст:
Распознавание Вещественного Числа из Строчки https://habr.com/ru/articles/757122/
Надеюсь, что этот текст поможет другим программистам микроконтроллеров решать повседневные задачи.
Я был бы признателен за набор тест-case(ов) для проверки данного программного компонента.
Cловарь
Акроним |
Расшифровка |
CSV |
Comma-separated values |
URL |
Uniform Resource Locator |
GNSS |
Global Navigation Satellite System |
API |
application programming interface |
NMEA |
National Marine Electronics Association |
Links, Cсылки
Распознавание Вещественного Числа из Строчки
ISO 26262-6 разбор документа (или как писать безопасный софт) https://habr.com/ru/articles/757216/
Комментарии (119)
sena
04.11.2023 20:46+72--В передаваемом тексте не должно быть самого символа разделителя как данных. Иначе конечный автомат заклинит.
В CSV вполне можно передавать поля с разделителем. Для этого поля заключаются в кавычки, как написано здесь https://en.wikipedia.org/wiki/Comma-separated_values
Leopotam
04.11.2023 20:46+11При этом допускается содержание самих кавычек внутри (через удвоение), а так же перевода строки внутри строкового значения (тоже через обертывание кавычками). Автор просто не стал писать валидный парсер и списал это на недостатки формата.
includedlibrary
04.11.2023 20:46+3Я правильно понимаю, что если мне надо больше двух значений извлечь, то придётся вызывать
csv_parse_textn раз?
BugM
04.11.2023 20:46+3Задача распасить csv уже решена тысячи раз. Чем ваше решение лучше любого другого из тех что есть на Гитхабе? Чем хуже понятно. 0 звезд Гитхаба, 0 истории использования. Вероятность критических ошибок больше чем в любом массовом решении написанном 10 лет назад.
Миру точно нужно еще одно решение уже давно решенной задачи?
aabzel Автор
04.11.2023 20:46-2Чем ваше решение лучше любого другого из тех что есть на Гитхабе?
В моем решении нет нужды в динамическом выделении памяти, что особенно важно для программирования микроконтроллеров.
В добавок к этому я представил не только код, но и документацию.
includedlibrary
04.11.2023 20:46+3Ваш код очень усложнён, вы зачем-то использовали конечный автомат и копирование, которое может быть не всегда нужно. Плюс для получения нескольких столбцов надо одну и ту же строку несколько раз сначала парсить, что неэффективно. Я за минут 30 накатал более красивый вариант:
csv.h:
#pragma once #include <stdint.h> #include <stdbool.h> typedef struct { char *column; uint32_t len; } csv_column_t; typedef struct { char *line; uint32_t line_size; uint32_t cur_pos; char sep; char repl_char; } csv_parser_t; void init_parser(csv_parser_t *parser, char *line, char sep); bool csv_next_column(csv_parser_t *parser, csv_column_t *column);csv.c:
#include "csv.h" #include <string.h> void init_parser(csv_parser_t *parser, char *line, char sep) { parser->line = line; parser->line_size = strlen(line); parser->cur_pos = 0; parser->sep = sep; } bool csv_next_column(csv_parser_t *parser, csv_column_t *column) { if(parser->cur_pos != 0) { // Восстанавливаем разделитель в строке parser->line[parser->cur_pos] = parser->repl_char; parser->cur_pos++; } column->column = &parser->line[parser->cur_pos]; column->len = 0; uint32_t start_pos = parser->cur_pos; for(; parser->cur_pos < parser->line_size; parser->cur_pos++) { // Если символ не является разделителем, идём дальше if(parser->line[parser->cur_pos] != parser->sep && parser->line[parser->cur_pos] != '\n' && parser->line[parser->cur_pos] != '\r') continue; // Временно заменяем разделитель на ноль column->len = parser->cur_pos - start_pos; parser->repl_char = parser->line[parser->cur_pos]; parser->line[parser->cur_pos] = 0; return true; } column->len = parser->cur_pos - start_pos; if(column->len != 0) { return true; } else if(parser->cur_pos > 0) { parser->line[parser->cur_pos - 1] = parser->repl_char; } return false; }Вот как с этим работать:
test.csv:
Name,Surname,Age,Gender John,Week,36,male Tesla,Killer,11,male Jane,Doe,34,female Liza,Silver,26,femalemain.c:
#include <stdio.h> #include <stdint.h> #include <stdbool.h> #include "csv.h" // Хотим вывести только имя и пол int main(void) { char buf[4096]; while(fgets(buf, sizeof(buf), stdin)) { csv_parser_t parser; init_parser(&parser, buf, ','); csv_column_t column; for(uint32_t i = 0; ; i++) { // если res = false, значит данных в сроке больше нет bool res = csv_next_column(&parser, &column); if(!res) break; if(i == 0 || i == 3) { // делаем что-то со столбцом } } } }aabzel Автор
04.11.2023 20:46-4Ваш код не соответствует требуемому API
includedlibrary
04.11.2023 20:46+5Ну и что? Что я могу сделать с вашим апи и не могу со своим? Я могу извлечь i-й столбец? Могу. Могу скопировать i-й столбец в другой буфер? Да. При этом я добавил возможность извлекать за раз больше одного столбца, у вас же надо каждый раз строку сначала разбирать
aabzel Автор
04.11.2023 20:46-4API очень важен. Ибо только через этот API код уже стыкуется с другими программными компонентами и модульными тестами.
Это как в отвертках. Есть только ограниченный набор допустимых шлицов.
includedlibrary
04.11.2023 20:46+5Ну так мой код довольно легко можно адаптировать и сделать такое же апи. Хотя, на мой взгяд, моё апи лучше.
aabzel Автор
04.11.2023 20:46-4Ваш код очень усложнён, вы зачем-то использовали конечный автомат и копирование, которое может быть не всегда нужно.
Дизайн на основе FSM как раз упрощает код, ибо все понимают, что есть такой паттерн, как он работает и как его использовать.
Тем более FSM надо использовать согласно ISO26262.includedlibrary
04.11.2023 20:46+1Мой код куда проще и понятнее вашего при этом ещё и эффективнее
aabzel Автор
04.11.2023 20:46-1Покажите лучше отчет после прогона модульных тестов для вашего кода. Вот набор тест-case(ов)
#include "test_csv.h" #include <stdint.h> #include <stdio.h> #include <string.h> #include "csv.h" #include "log.h" #include "unit_test_check.h" bool test_csv_cnt(void){ LOG_INFO(TEST, "%s():", __FUNCTION__); bool res = true; set_log_level(CSV, LOG_LEVEL_DEBUG); EXPECT_EQ(1, csv_cnt("", ';') ); EXPECT_EQ(2, csv_cnt(";", ';') ); EXPECT_EQ(1, csv_cnt("a", ';') ); EXPECT_EQ(3, csv_cnt(";;", ';') ); EXPECT_EQ(4, csv_cnt(",,,", ',') ); EXPECT_EQ(1, csv_cnt("4452", ';') ); EXPECT_EQ(6, csv_cnt("7354, 1105.000,20.75, 16:07:44, 14/7/2023, 1517952720", ',') ); EXPECT_EQ(3, csv_cnt("ll wm8731 debug;ll i2c debug;tsr 127", ';') ); set_log_level(CSV,LOG_LEVEL_INFO); return res; } bool test_csv_parse_text(void){ LOG_INFO(TEST, "%s():", __FUNCTION__); bool res = true; set_log_level(CSV, LOG_LEVEL_DEBUG); char sub_text[80] = ""; memset(sub_text, 0, sizeof(sub_text)); EXPECT_TRUE( csv_parse_text("",';', 0, sub_text, sizeof(sub_text))); EXPECT_STREQ("", sub_text); EXPECT_TRUE( csv_parse_text("h",';', 0, sub_text, sizeof(sub_text))); EXPECT_STREQ("h", sub_text); EXPECT_TRUE( csv_parse_text(";;c;;",';', 2, sub_text, sizeof(sub_text))); EXPECT_STREQ("c", sub_text); EXPECT_TRUE( csv_parse_text("ll wm8731 debug;ll i2c debug;tsr 127",';', 0, sub_text, sizeof(sub_text)) ); EXPECT_STREQ("ll wm8731 debug", sub_text); EXPECT_TRUE( csv_parse_text("ll wm8731 debug;ll i2c debug;tsr 127",';', 1, sub_text, sizeof(sub_text)) ); EXPECT_STREQ("ll i2c debug", sub_text); EXPECT_TRUE( csv_parse_text("ll wm8731 debug;ll i2c debug;tsr 127",';', 2, sub_text, sizeof(sub_text)) ); EXPECT_STREQ("tsr 127", sub_text); set_log_level(CSV, LOG_LEVEL_INFO); return res; }includedlibrary
04.11.2023 20:46+1С удовольствием, дайте мне тесты, я вам предоставлю
aabzel Автор
04.11.2023 20:46-1#include "test_csv.h" #include <stdint.h> #include <stdio.h> #include <string.h> #include "csv.h" #include "log.h" #include "unit_test_check.h" bool test_csv_cnt(void){ LOG_INFO(TEST, "%s():", __FUNCTION__); bool res = true; set_log_level(CSV, LOG_LEVEL_DEBUG); EXPECT_EQ(1, csv_cnt("", ';') ); EXPECT_EQ(2, csv_cnt(";", ';') ); EXPECT_EQ(1, csv_cnt("a", ';') ); EXPECT_EQ(3, csv_cnt(";;", ';') ); EXPECT_EQ(4, csv_cnt(",,,", ',') ); EXPECT_EQ(1, csv_cnt("4452", ';') ); EXPECT_EQ(6, csv_cnt("7354, 1105.000,20.75, 16:07:44, 14/7/2023, 1517952720", ',') ); EXPECT_EQ(3, csv_cnt("ll wm8731 debug;ll i2c debug;tsr 127", ';') ); set_log_level(CSV,LOG_LEVEL_INFO); return res; } bool test_csv_parse_text(void){ LOG_INFO(TEST, "%s():", __FUNCTION__); bool res = true; set_log_level(CSV, LOG_LEVEL_DEBUG); char sub_text[80] = ""; memset(sub_text, 0, sizeof(sub_text)); EXPECT_TRUE( csv_parse_text("",';', 0, sub_text, sizeof(sub_text))); EXPECT_STREQ("", sub_text); EXPECT_TRUE( csv_parse_text("h",';', 0, sub_text, sizeof(sub_text))); EXPECT_STREQ("h", sub_text); EXPECT_TRUE( csv_parse_text(";;c;;",';', 2, sub_text, sizeof(sub_text))); EXPECT_STREQ("c", sub_text); EXPECT_TRUE( csv_parse_text("ll wm8731 debug;ll i2c debug;tsr 127",';', 0, sub_text, sizeof(sub_text)) ); EXPECT_STREQ("ll wm8731 debug", sub_text); EXPECT_TRUE( csv_parse_text("ll wm8731 debug;ll i2c debug;tsr 127",';', 1, sub_text, sizeof(sub_text)) ); EXPECT_STREQ("ll i2c debug", sub_text); EXPECT_TRUE( csv_parse_text("ll wm8731 debug;ll i2c debug;tsr 127",';', 2, sub_text, sizeof(sub_text)) ); EXPECT_STREQ("tsr 127", sub_text); set_log_level(CSV, LOG_LEVEL_INFO); return res; }includedlibrary
04.11.2023 20:46+1Кое-что моя реализация не может - работать с read only строками (не думаю, что это прямо проблема, потому что вы всё равно из внешнего источника csv получаете). За 30 минут сделать не вышло, тут вы меня подловили, но я развлёкся зато. Вот исправленная реализация, проходящая все ваши тесты (плюс бонус - я реализовал ваше апи через своё).
csv.h:
#pragma once #include <stdint.h> #include <stdbool.h> typedef struct { char *column; uint32_t len; } csv_column_t; typedef struct { char *line; uint32_t line_size; uint32_t cur_pos; char sep; char repl_char; bool is_first; bool is_last; } csv_parser_t; void init_parser(csv_parser_t *parser, char *line, char sep); bool csv_next_column(csv_parser_t *parser, csv_column_t *column); void csv_restore_line(csv_parser_t *parser);csv.c:
#include "csv.h" #include <string.h> void init_parser(csv_parser_t *parser, char *line, char sep) { parser->line = line; parser->line_size = strlen(line); parser->cur_pos = 0; parser->repl_char = 0; parser->is_first = true; parser->is_last = false; parser->sep = sep; } bool csv_next_column(csv_parser_t *parser, csv_column_t *column) { if(!parser->is_first) { // Восстанавливаем разделитель в строке parser->line[parser->cur_pos] = parser->repl_char; parser->repl_char = 0; parser->cur_pos++; } else { parser->is_first = false; } column->column = &parser->line[parser->cur_pos]; column->len = 0; uint32_t start_pos = parser->cur_pos; for(; parser->cur_pos < parser->line_size; parser->cur_pos++) { // Если символ не является разделителем, идём дальше if(parser->line[parser->cur_pos] != parser->sep && parser->line[parser->cur_pos] != '\n' && parser->line[parser->cur_pos] != '\r') { parser->is_last = true; continue; } // Временно заменяем разделитель на ноль column->len = parser->cur_pos - start_pos; parser->repl_char = parser->line[parser->cur_pos]; parser->line[parser->cur_pos] = 0; return true; } column->len = parser->cur_pos - start_pos; if(column->len != 0) return true; if(parser->is_last) { if(parser->cur_pos > 0) parser->line[parser->cur_pos - 1] = parser->repl_char; return false; } else { parser->is_last = true; return true; } } void csv_restore_line(csv_parser_t *parser) { if(!parser->is_first) { parser->line[parser->cur_pos] = parser->repl_char; } }test.c:
#include <stdint.h> #include <stdio.h> #include <string.h> #include <assert.h> #include "csv.h" uint32_t csv_cnt(char *line, char sep) { csv_parser_t parser; init_parser(&parser, line, sep); uint32_t count; for(count = 0; ; count++) { csv_column_t column; if(!csv_next_column(&parser, &column)) break; } return count; } #define CNT_TEST(str, sep, eq) do { \ char buf[] = str; \ assert(csv_cnt(buf, sep) == eq); \ assert(!strcmp(buf, str)); \ } while(0) void test_csv_cnt(void) { CNT_TEST("", ';', 1); CNT_TEST(";", ';', 2); CNT_TEST("a", ';', 1); CNT_TEST(";;", ';', 3); CNT_TEST(",,,", ',', 4); CNT_TEST("4452", ';', 1); CNT_TEST("7354, 1105.000,20.75, 16:07:44, 14/7/2023, 1517952720", ',', 6); CNT_TEST("ll wm8731 debug;ll i2c debug;tsr 127", ';', 3); } bool csv_parse_text(char *line, char sep, uint32_t index, char *out, uint32_t size) { csv_parser_t parser; init_parser(&parser, line, sep); for(uint32_t i = 0; ; i++) { csv_column_t column; if(!csv_next_column(&parser, &column)) return false; if(i != index) continue; if(size <= column.len) return false; memcpy(out, column.column, column.len + 1); csv_restore_line(&parser); return true; } } #define PARSE_TEST(str, sep, idx, out_buf, res, column) \ do { \ char buf[] = str; \ assert(csv_parse_text(buf, sep, idx, out_buf, sizeof(out_buf)) == res); \ assert(!strcmp(out_buf, column)); \ assert(!strcmp(str, buf)); \ } while(0) void test_csv_parse_text(void) { char column[80]; PARSE_TEST("", ';', 0, column, true, ""); PARSE_TEST("h", ';', 0, column, true, "h"); PARSE_TEST(";;c;;", ';', 2, column, true, "c"); PARSE_TEST("ll wm8731 debug;ll i2c debug;tsr 127", ';', 0, column, true, "ll wm8731 debug"); PARSE_TEST("ll wm8731 debug;ll i2c debug;tsr 127", ';', 1, column, true, "ll i2c debug"); PARSE_TEST("ll wm8731 debug;ll i2c debug;tsr 127", ';', 2, column, true, "tsr 127"); } int main(void) { test_csv_cnt(); test_csv_parse_text(); }
aabzel Автор
04.11.2023 20:46Считаю что Вам понравится вот этот текст
51 Атрибут Хорошего С-кода (Хартия Си программистов)
https://habr.com/ru/articles/679256/
BugM
04.11.2023 20:46Возьмем первое найденное решение на Гитхабе у которого звездочек побольше.
https://github.com/rgamble/libcsv
Сотня звездочек есть. Работает вероятно быстрее вашего. Коду уже много-много лет. Вероятность что в нем есть ошибки стремится к нулю. Код выглядит проще чем у вас. Документация тоже лучше чем у вас.
И зачем? Не, ну если это лаба студента я понимаю. Таких лабораторок тоже тысячи в год пишутся.
aabzel Автор
04.11.2023 20:46-7В этом коде есть множественный return. Это запрещено стандартом ISO26262
Рекомендую почитать этот текст
ISO 26262-6 разбор документа (или как писать безопасный софт)
https://habr.com/ru/articles/757216/
BugM
04.11.2023 20:46+10Вы никогда не слышали про концепцию fail fast? Функции проверяющие что-то в начале работы и быстро выходящие если это не так читаются заметно легче.
А вот монстрообразные вложенные ифы чтобы дотащить все ветки до вашего единственного return читаются очень тяжело. Я бы на код ревью зарубил такое.
SpiderEkb
04.11.2023 20:46+1Концепция сама по себе неплоха, но рискованная т.к. к моменту когда "что-то пошло не так и нужно выйти" функция уже может сделать что-то такое, что требует подчистки за собой. И для этого существует более безопасная концепция "единой точки выхода". К сожалению, в С, красиво не реализуется иначе чем через GOTO (в С++ это реализуется через исключения).
В некоторых языках для это реализован специальный блок on-exit [err_flag], в который попадаем после return (и можем там изменить возвращаемое значение) или при возникновении ошибки-исключения (тогда, если указан err_flag, он будет взведен.
Есть языки, поддерживающие "сабрутины" (subroutine) - подпрограммы (как в бейсике) внутри функций. Там можно реализовывать единую точку выхода через такую сабрутину - вместо return вызывается она, там выполняются все необходимые действия и после этого уже делается return.
В любом случае, единая точка выхода является более безопасной реализацией.
PrinceKorwin
04.11.2023 20:46+1Как концепция "единой точки выхода" защитит меня от забывчивости забыть освободить ресурсы?
SpiderEkb
04.11.2023 20:46От склероза не защитит ничего. Тут только голову электричеством лечить.
Но концепция единой точки выхода позволит сделать код более компактным и понятным в ситуации, когда перед return нужно сделать что-то еще (даже дублировать 3-4 строки кода перед каждым return уже не только накладно, но еще повышает риск в 2-х местах поставить их, а в третьем забыть или пропустить по невнимательности).
Так же в случае модификации кода когда перед выходом нужно добавить какое-то действие снижается риск того, что где-то его добавите, а где-то пропустите.
includedlibrary
04.11.2023 20:46+1Вы описали ситуацию, где надо перед возвратом что-то делать, но в нашем же случае это не так. Зачем тогда слепо следовать правилу одной точки выхода? Если надо что-то делать перед возвратом, соглашусь должна быть одна точка выхода, если нет, то не вижу проблемы в множественных точках возврата.
aabzel Автор
04.11.2023 20:46-1Множественный return запрещен стандартом ISO26262
Рекомендую почитать этот текст
ISO 26262-6 разбор документа (или как писать безопасный софт)https://habr.com/ru/articles/757216/includedlibrary
04.11.2023 20:46+1Это говорит только о том, что стандарты должны меняться, когда становится понятно, что некоторые вещи в них необоснованны и странны. Но если вам строго надо следовать стандарту, тут не попишешь, придётся одну точку выхода делать.
SpiderEkb
04.11.2023 20:46Это еще может говорить о том, что если для нас что-то неочевидно или интуитивно непонятно, то это совершенно необязательно неправильно.
includedlibrary
04.11.2023 20:46+1Приведите пример, где множественный return делает код небезопасным, и при этом нам не нужно ничего делать при выходе из функции. Если же нам надо что-то делать при выходе из функции, чем плох goto? Какую угрозу безопасности он несёт в таком случае?
includedlibrary
04.11.2023 20:46+1В софте, от которого зависят жизни, наверное стоит. Правда, мне в голову только такой вариант приходит, чтобы не нагромождать вложенные ифы:
int func(void) { int err = action_1(); if(err != 0) return cleanup_func(err, arg1, arg2, ...); // какой-то код err = action_2(); if(err != 0) return cleanup_func(err, arg1, arg2, ...); //какой-то код return 0; }Ну или везде использовать фигурные скобки, тогда ситуация, которая была у apple с меньшей вероятностью произойдёт.
BugM
04.11.2023 20:46+1В софте, от которого зависят жизни, наверное стоит
Уже есть куча статей про автомобильный софт. Там страх и ужас. Типичный хайлоад сервис с девятками надежности написан лучше.
Зато в автомобильном софте хорошо умеют подстилать соломки. Вотчдоги на все, вообще на все. Действие по умолчанию когда все сломалось нормальное. Перезагружается все само, быстро и инициализируется нормальным состоянием. И все такое. Тут типичному хайлоад сервису с девятками стоило бы поучиться.
SpiderEkb
04.11.2023 20:46Тут уже складывается ощущение, что "любой костыль, лишь бы сохранить несколько точке выхода".
Это хорошо когда то, что подлежит зачистке, лежит в глобальной области видимости и доступно из cleanup_func, а не в локальной области func.
А что бы будете делать, если у вас в процессе выполнения func открывается пара-тройка файлов (которые нужно закрыть на выходе), блокируются какие-то записи (которые нужно разблокировать перед выходом), динамически выделяется временная память, которую нужно освободить?
При этом код
int func(void) { int err = some_action_1(); if(err == 0) err = some_action_2(); if(err == 0) err = some_action_3(); // освобождаем локально выделенные ресурсы //... // если была ошибка и нужен откат if (err != 0) rollback_func(); return err; }не выглядит такой уж "лапшой" и не содержит многоуровневой вложенности. Но при этом просто лаконичнее.
iig
04.11.2023 20:46+1А что бы будете делать, если у вас в процессе выполнения func открывается пара-тройка файлов (которые нужно закрыть на выходе), блокируются какие-то записи (которые нужно разблокировать перед выходом), динамически выделяется временная память, которую нужно освободить?
Деструкторы в C++ приблизительно это и должны делать же. Если очень нужно именно C - не забываем инициализировать; подпираем костылем:
if (file1) {fclose(file1); file1 = NULL;}; if (memory) {free(memory); memory=NULL;};
SpiderEkb
04.11.2023 20:46Деструкторы в C++ приблизительно это и должны делать же
В С++ единая точка входа реализуется исключениями. Просто и красиво - возникла ошибка - кидаем исключение и попадаем в ту самую "единую точку выхода".
Если очень нужно именно C - не забываем инициализировать; подпираем костылем
И так 100500 раз на каждый return? О чем и говорю - лютый костылинг и дикое дублирование кода. Он просто становится замусоренным и слабочитаемым.
О чем, собственно, и речь - множественные return удовлетворительно работают в очень простых и небольших функциях. И всегда есть риск того, что если потребуется усложнить логику функции, то сразу возникнет куча проблем.
Единая точка выхода более универсальна - я показывал как это работает на простых функциях и точно также это будет работать и на более сложных. Т.е. этот подход более устойчив к изменениям в сторону усложнения логики.
Мне ни в коей мере не хочется сводит все это к холивару и поливанию друг друга разными дурно пахнущими жидкими субстанциями. Просто высказал свою точку зрения, сформированную многолетним опытом разработки в разных областях и на разных языках.
iig
04.11.2023 20:46+3О чем, собственно, и речь - множественные return удовлетворительно работают в очень простых и небольших функциях.
Да. Есть даже паттерн такой - простые небольшие функции ;)
includedlibrary
04.11.2023 20:46-1Уж лучше goto, чем исключения, как по мне. goto ведёт всегда в одну точку причём в той же функции, а исключение ведёт к месту обработки. Забыл обработать нужный класс исключений в своей функции, тебя выбросит в другую, а в худшем случае вся программа упадёт
includedlibrary
04.11.2023 20:46-1Если функция большая, её, конечно, нужно разбить на несколько простых. Но если она маленькая, то ради единой точки выхода, правда, нужно создавать ещё три функции? Это очень усложняет чтение, потому что вместо одной функции на 20 строк, у вас одна функция, в которой вызывается три других, и вам надо посмотреть, что делает каждая. Да это лапшой не назовёшь, но воспринимается такой код сложнее
aabzel Автор
04.11.2023 20:46-2Код надо писать так чтобы каждая функция помещалась на один экран (максимум 45строк)
includedlibrary
04.11.2023 20:46+1В моём комменте я говорил о функции длиной 20 строк, покажите, где я говорил, что нужно писать громоздкие функции? Вот моя цитата, которая об обратном говорит.
Если функция большая, её, конечно, нужно разбить на несколько простых.
SpiderEkb
04.11.2023 20:46+1Ну, во-первых, лучше везде придерживаться одного стиля, а единая точка выхода более универсальна.
Во-вторых, то что вы сейчас написали функцию, где перед выходом ничего не надо делать, совершенно не гарантирует того, что через месяц вам ее не придется модифицировать до состояния когда перед выходом что-то надо будет сделать. Зачем самому себе закладывать потенциальные сложности?
SpiderEkb
04.11.2023 20:46-1Как пример из языка, где поддерживаются оба механизма:
on-exit; if curOpnd; exec sql close curRDKCHK1Clients; curOpnd = *off; endif; end-proc;Тут перед выходом проверяется открыт ли sql курсор и если его успели открыть до выхода - нужно закрыть.
В это блок автоматически попадем при return где бы он ни был.
Второй вариант:
BegSr srExit; if error <> *blanks; needLogError = *on; MQlogMSG = %trim(Message_Text(error)); RAMSGTYPE = 'CAFZ01ERR'; clear rtnCode; endif; return; endsr;Проверяется была ли ошибка и если была, выполняются некоторые действия.
Тут вместо return делаем exsr srExit;
В любой реализации, нам не надо для каждого выхода прописывать одни и те же действия. И если потребуется модифицировать код так, что список действий на выходе потребует расширения или изменения, сделать это надо будет в одном месте а не шарахаться по всему коду в поисках "а где там у нас еще точки выхода были" с риском что-то проглядеть.
BugM
04.11.2023 20:46+1Ну сделайте рядом метод clearAfterMyFunc. Будет у вас единая точка очистки ресурсов.
aabzel Автор
04.11.2023 20:46-1Вы никогда не слышали про концепцию fail fast? Функции проверяющие что-то в начале работы и быстро выходящие если это не так читаются заметно легче.
Что Вы мне то это объясняете?
Скажите это чиновникам, которые составили стандарт ISO-26262.
https://habr.com/ru/articles/757216/BugM
04.11.2023 20:46Чиновники часто не очень понимают что делают. Это нормально.
Если где-то вместо нормальных правил написания кода, это любые стайлгайды от любой крупной продуктовой конторы, заставляют использовать писанину от чиновников то бежать оттуда надо.
aabzel Автор
04.11.2023 20:46Вообще то вся мировая разработка Automotive работает по ISO26262.
BugM
04.11.2023 20:46+1Ну-ну. Вы сами сертификацию проходили?
aabzel Автор
04.11.2023 20:46Automotive компилятор С (например GHS) даже не соберет код, где в функции 2 return(а).
Там сертификация встроена прямо в ядро Си-компилятора.
includedlibrary
04.11.2023 20:46+2Один return без goto приводит к такой лапше из вложенных ифов, что приводит к увеличению вероятности ошибки и усложнению понимания работы кода. Странный стандарт
includedlibrary
04.11.2023 20:46+1Вот вы бы вместо минуса привели бы лучше пример, который доказывает мою неправоту. Доупстим есть функция
int func(void) { int err = some_action_1(); if(err != 0) { // обрабатываем ошибку, // возвращаем err return err; } // делаем что-то полезное, // потом снова вызываем функцию, // которая может вернуть ошибку err = some_action_2(); if(err != 0) { // обрабатываем ошибку, // возвращаем err return err; } // делаем что-то ещё, возвращаем 0 return 0; }Перепишите её так, чтобы точка выхода была одна, и код не стал лапшой.
SpiderEkb
04.11.2023 20:46+1int func(void) { int err = some_action_1(); if(err == 0) err = some_action_2(); if(err == 0) err = some_action_3(); // делаем что-то ещё // ... return err; }
includedlibrary
04.11.2023 20:46-1А если нужна именно последовательность, как я описал. То есть после some_action_1 должен идти какой-то код, после вызыватся some_action_2 и идти следующий код. Или вы предлагаете, код, который идёт после some_action_1 и some_action_2 в отдельные функции запихнуть?
void after_some_action_1(void) { // код } void after_some_action_2(void) { // код } int func(void) { int err = some_action_1(); if(err == 0) { after_some_action_1(); err = some_action_2(); } if(err == 0) after_some_action_2(); return err; }Теперь вложенных ифов, конечно, нет. Но код стал сложнее для восприятия. Обосновывается же это только странными правилами стандарта, а не реальными причинами.
aabzel Автор
04.11.2023 20:46-1
lair
04.11.2023 20:46+2Как хорошо, что мне не надо писать код по этому стандарту.
(я подозреваю, что если пришлось бы, я бы нашел тулинг, который берет код, написанный не по стандарту, а потом его преобразует)
mentin
04.11.2023 20:46+1Смотря на строчку avoid global variables or justify their usage вспоминается отчёт о коде Тойоты, полученный когда её судили жертвы "внезапного ускорения". Тойота его пыталась списать на застрявшую педаль, но причиной был софт конечно. Мешанина из сотен глобальных переменных, с доступом из разных потоков, меня впечатлила.
aabzel Автор
04.11.2023 20:46вспоминается отчёт о коде Тойоты, полученный когда её судили жертвы "внезапного ускорения". Тойота его пыталась списать на застрявшую педаль, но причиной был софт конечно.
Вот-вот ISO26262 написан кровью!
makkarpov
04.11.2023 20:46+1Более правильный вывод - ISO26262 и кровь абсолютно никак между собою не связаны. Точнее, хорошо, если там не окажется положительной корреляции. Потому что предполагаю, что наличие искусственных ограничений порождает более громоздкие конструкции, которые сложнее анализировать и верифицировать. А примитивность правил напоминает скорее религию и карго-культ, чем реальную попытку увеличить надежность чего-то там.
Ваш код и пример выше этот тезис прекрасно демонстрируют - нет двойных return, все переменные освящены православным священником, зато есть повреждение памяти и потенциальное удаленное исполнение произвольного кода (кто-то настраивает MPU в своих проектах?), если вдруг вход будет злонамеренным.
И бонусом уже отвлеченный вопрос - а вы разрабатываете софт для автомобилей? Если да - то для каких именно?
mentin
04.11.2023 20:46+3Да, думаю корреляция положительная - потому что стандарты вроде ISO26262 в реальности используют не для улучшения качества, а для снятия с себя с ответственности - когда их будут судить, будут отбиваться что мол пользуются лучшими стандартами индустрии. Для улучшения же качества нужны не дурные жесткие стандарты, а реальное понимание разработчиками что они делают, и желание подумать о потенциальных ошибках.
Так и тут - single return помог автору неправильно обработать нехватку буфера out_buff, возвращая true вместо false. Хороший разработчик забил бы на single return, и не сделал бы этой ошибки. Зато стоило бы написать fuzzer test в пять строчек, и тут же нашелся бы buffer overflow.
aabzel Автор
04.11.2023 20:46Исправил, Обновил. Теперь этот тест проходит.
bool test_csv_parse_text_overflow(void){ LOG_INFO(TEST, "%s():", __FUNCTION__); bool res = true; set_log_level(CSV, LOG_LEVEL_DEBUG); char sub_text[5] = ""; memset(sub_text, 0, sizeof(sub_text)); ASSERT_FALSE( csv_parse_text("aa;123456789;cc",';', 1, sub_text, sizeof(sub_text)) ); set_log_level(CSV, LOG_LEVEL_INFO); return res; }
makkarpov
04.11.2023 20:46+2После использования в коде strcpy (за который даже студентов бьют по рукам) довольно странно рассуждать про вред двойного return. Тут вы его проверили, но раз вы уже посчитали длину - можно было использовать memcpy.
P.S. А, там еще помимо strcpy (который корректный, но в целом считается плохой практикой) есть переполнение temp, которое уже нигде не проверяется. Про тихие "return true" при ошибках вам уже сказали.
Норм, код некорректный, небезопасный, нечитаемый, зато без двойного return.
aabzel Автор
04.11.2023 20:46-2Норм, код некорректный, небезопасный,
Предоставьте тест-case, который покажет в run-time, что код из текста не работает.
mentin
04.11.2023 20:46+10Жесть код.
Копирование в temp не проверяет на переполнение - надеясь на то что любой CSV будет соблюдать вашу магическую константу CSV_VAL_MAX_SIZE.
Само это копирование идет постоянно, csv_cnt_acc_proc даже не смотрит на fetch_index, копирует все значения подряд, видимо чтобы всё замедлить, другого смысла не вижу.
Зачем там вообще этот temp, проще же запомнить адрес начала текущего значения и ничего не копировать, не нужен огромный буфер temp, не нужна магическая константа, не нужно бесполезное копирование всего увиденного текста.
Возвращая значение, размер буфера уже проверяется, но при переполнении всего-навсего делается логгинг, и возвращается
true, как будто всё в порядке.И это копирование ровно так же не нужно, можно просто вернуть string_view или аналог, указывающий на буфер.
Вы понимаете, что нормальный разбор CSV, со строгой поддержкой кавычек и переводов строки, может работать на порядки быстрее написанного, не говоря уже о надежности?
aabzel Автор
04.11.2023 20:46-3Жесть код.
Предоставьте тест-case, который покажет в run-time, что код из текста не работает.
mentin
04.11.2023 20:46+12Переполнение буфера проблемой не считаете? Или в коде есть хоть одна проверка на CSV_VAL_MAX_SIZE?
P.S. обижаться и минусовать карму за детальнейший code review - фи. Первое чему стоит научиться разработчику - это ценить любой code review, а не воспринимать критику кода в штыки.
mentin
04.11.2023 20:46+1А чтобы показать работает или нет, нужна спецификация. Дан же прототип функции, а что за bool она возвращает - не сказано. По мне, невозможность различить по результату функции, кроме анализа логов, две ситуации - вернули ли нам значение из CSV, или не хватило переданного буфера и out_buff остался нетронутым - большая проблема, а код в обоих случаях вернет true. Но поскольку спецификация не говорит, что мы возвращаем, то можно сказать всё ок :)
aabzel Автор
04.11.2023 20:46Дан же прототип функции, а что за bool она возвращает - не сказано.
В случае какой-либо ошибки (например выход за пределы предоставленной для sub_str памяти или неверном индексе) вернуть false.mentin
04.11.2023 20:46+1Тогда тест-кейс описанный выше - после LOG_ERROR в csv_proc_fetch_value вернётся true, хотя ожидалось false.
aabzel Автор
04.11.2023 20:46Копирование в temp не проверяет на переполнение - надеясь на то что любой CSV будет соблюдать вашу магическую константу CSV_VAL_MAX_SIZE.
Исправил, код обновил.
makkarpov
04.11.2023 20:46+2
А копипаст кучи идентичных кусков кода вместо адекватной декомпозиции - это тоже требование ISO-26262?
aabzel Автор
04.11.2023 20:46Само это копирование идет постоянно, csv_cnt_acc_proc даже не смотрит на fetch_index, копирует все значения подряд, видимо чтобы всё замедлить, другого смысла не вижу.
Зачем там вообще этот temp, проще же запомнить адрес начала текущего значения и ничего не копировать, не нужен огромный буфер temp, не нужна магическая константа, не нужно бесполезное копирование всего увиденного текста.
Возвращая значение, размер буфера уже проверяется, но при переполнении всего-навсего делается логгинг, и возвращается
true, как будто всё в порядке.
Исправил. Обновил текст.
SpiderEkb
04.11.2023 20:46+3К сожалению, тут реализация заведомо ограничена неудачным контрактом.
На деле же просится, во-первых, полный разбор строки за один проход, во-вторых, попутный анализ потенциального типа каждого элемента. Например, если там не содержится ничего, кроме знаков +/-, десятичного разделителя и цифр - это потенциально число (может быть преобразовано в число). Иначе - строка. Также (при необходимости) можно распознавать потенциальные дату, время.
Если говорить конкретно о NMEA, то встречались простые, быстрые и компактные парсеры на С (NMEAlib, NMEA...) где автоматически распознается тип строки и сразу возвращается заполненная соответствующая структура.
SpiderEkb
04.11.2023 20:46+2Но даже при таком контракте можно что-то выморщить - если сценарий использования подразумевает множественный вызов для одной строки с извлечением 1-го, 2-го и далее элементов, можно на первом вызове сделать полный разбор с занесением всех элементов во внутренний статический массив, а для всех последующих (если строка та же), сразу возвращать соотв. элемент массива.
Как вариант, не хранить элементы, но только разметку - позиции начала каждого элемента в строке. Тогда на втором и последующем вызовах для данной строки не потребуется заново проходить по всей строке, достаточно просто вытащить из нее кусок от начала нужного элемента до начала следующего за ним.
aabzel Автор
04.11.2023 20:46-1На деле же просится, во-первых, полный разбор строки за один проход,
Дело в том, что нельзя использовать динамическое выделение памяти. А в CSV строке может быть десятки элементов. Поэтому на заранее не известно какого размера нужен массив, чтобы складировать результат.
iig
04.11.2023 20:46+1в CSV строке может быть десятки элементов.
Количество элементов в протоколе NMEA известно же?
SpiderEkb
04.11.2023 20:46+3Вообще-то, насколько я понимаю, речь идет не о CSV строках "ва-аще", а о каком-то конкретном случае, когда формат строки в целом известен.
И я очень сомневаюсь, что контроллер, где недоступно динамическое выделение памяти, будет работать с CSV строками в несколько мегабайт и тысячами элементов.
На 99.9% уверен, что есть заранее известный формат CSV строки (будь то NMEA поток или какие-то служебные сообщения) который нужно разбирать. И в условиях жесткого ограничения ресурсов первый путь оптимизации делать не универсальное решение на все случаи жизни, но конкретное решение под конкретную задачу с учетом все е особенностей.
А решение каждый раз елозить по всей строке в поисках нужного элемента... Ну мне, по крайней мере, это кажется лютым костылингом и полным отсутствием сколь-либо продуманной архитектуры решения.
Если там речь пойдет о пресловутых мегабайтных строках с тысячами элементов, то извлечение 763-го элемента в такой реализации будет занимать дико много времени.
И даже в таком случае я бы постарался как-то оптимизироваться. Например, запоминать номер и позицию последнего выбранного элемента в строке и следующий поиск начинать от него - нужен предыдущий - идем назад (или смотрим будет он ближе к последнему или к началу строки), нужен следующий - идем дальше от последнего обработанного. А если там вообще последовательно (1, 2, 3,...) и других сценариев нет - совсем просто. Запомнили что для этой строки в прошлый раз извлекали второй элемент, строка та же, нужен третий - значит идем по строке с того места, где в прошлый раз остановились.
Короче говоря, нужно всегда учитывать как контракт, так и сценарий его использования и от этого строить реализацию.
iig
04.11.2023 20:46Автор, похоже, хотел натянуть сову парсинга csv на глобус конечных автоматов ;)
Если нужно парсить данные gps-приемника, формат которых известен заранее - что мешает, раз уж есть strcpy, взять strtok, или сразу scanf?
inkelyad
04.11.2023 20:46+1Автор, похоже, хотел натянуть сову парсинга csv на глобус конечных автоматов ;)
И написать его вручную... Засунуть грамматику в генератор парсеров и получить весь этот конечный автомат автоматически - наверное, адекватнее было бы.
iig
04.11.2023 20:46Ладно бы regexp'ы нужно было парсить. Но для выбранной задачи это как пушкой по воробьям, только не стрелять а ездить.
aabzel Автор
04.11.2023 20:46Засунуть грамматику в генератор парсеров и
Что можно почитать про генераторы парсеров? Что это за технология такая?
includedlibrary
04.11.2023 20:46+1Вы пишите файл с грамматикой, генератор парсеров на его основе генерирует код для синтаксического разбора языка, определяесого заданной грамматикой. Почитайте про bison, например.
SpiderEkb
04.11.2023 20:46+1Я старый и немодный, в мое время это были LEXX и YACC (в виндовых версиях FLEX и BISON).
Может сейчас чего помощнее и посовременнее есть.
Тулзы, позволяющие разрабатывать парсер и лексический анализатор. Можно свой скриптовый язык сделать +я делал, даде работало, давно, правда, было...)
aabzel Автор
04.11.2023 20:46А они (BISON и FLEX) могут сделать парсер для бинарного протокола c контрольной суммой (типа ModBus)?
lair
04.11.2023 20:46+2Короткое гугление показывает, что ModBus - это протокол, а не формат (или язык). Для него нет грамматики. Значит, нет и парсера.
SpiderEkb
04.11.2023 20:46+1В общем, да.
Lex — это инструмент для лексического анализа, который может использоваться для выделения из исходного текста определенных строк заранее заданным способом. Yacc — это инструмент для грамматического разбора; он читает текст и может использоваться для конвертирования последовательности слов в структурированный формат для дальнейшей обработки.
SpiderEkb
04.11.2023 20:46+1Не скажу, не уверен. Насколько помню, там все строилось на описании грамматики языка.
Может быть как-то можно туда бинарное накрутить, но проще руками (как мне кажется).
Погуглите лексс и якк просто для понимания что там и как
includedlibrary
04.11.2023 20:46+2Вам может подойти kaitai struct, там как раз можно описать на декларативном языке бинарный формат, и он сгенерирует вам код, который с этим форматом работает.
SpiderEkb
04.11.2023 20:46+1Ошибся по старости лет LEX с одной X
Разновидности - FLEX (Fast LEX) и GNU BISON
Но это именно про тексты.
lair
04.11.2023 20:46ABNF уже есть даже: https://datatracker.ietf.org/doc/html/rfc4180#section-2
(BTW, этот RFC явным образом описывает поведение для полей, содержащих разделители, так что парсер из поста этот RFC не реализует)
vasyakolobok77
04.11.2023 20:46+5CSV это просто последовательность символов, которые разделены запятой или любым другим одиночным символом
Позвольте не согласиться. Как уже отметили другие, в формате CSV есть особые правила для обработки многострочных значений и значений, содержащих спецсимволы (в том числе и сам разделитель), такие значения оборачиваются в кавычки. Наивные парсеры не учитывают такие спец. значения.
Потом, любой URL это, в сущности, та же самая пресловутая CSV строчка
Опять же, не все так просто https://en.wikipedia.org/wiki/Uniform_Resource_Identifier и в общем случае идентификатор ресурса зависит от схемы, имеет в себе компоненты (scheme, userinfo, host, port, path, query, fragment), и разделителями компонент являются разные группы символов.
К чему это я? Если есть возможность использовать готовую отлаженную библиотеку, то лучше использовать ее, нежели изобретать еще один велосипед на квадратных колесах.
lair
04.11.2023 20:46+1Как это обычно и бывает в программировании все задачи решаются золотым шаблоном: конечным автоматом.
Нет в программировании "золотых шаблонов", как и серебрянных пуль.
А конкретно у вас - не синтаксический разбор CSV-строчек (потому что, как вам уже написали, вы не разбираете типовый для CSV случай вложенного разделителя), а решение какой-то частной нужной вам задачи, которую вы потом пытаетесь выдать за общую.
Мне много раз приходилось загружать данные из CSV (но ни разу не пришлось писать парсер), и контракт "Также дан индекс элемента в виде положительного целого числа [...] Вернуть подстроку, которая соответствует индексу" мне не был нужен никогда - если у меня есть CSV, мне почти всегда нужно больше одного значения оттуда, поэтому строка должна парситься один раз, а не столько раз, сколько мне из нее нужно значений.
SpiderEkb
04.11.2023 20:46+1мне почти всегда нужно больше одного значения оттуда, поэтому строка должна парситься один раз, а не столько раз, сколько мне из нее нужно значений
Ну на самом деле так оно и есть. Но если ресурсы ограничены и нет возможности вывалить весь массив целиком, то почему бы не сделать последовательный парсинг - если передана новая строка, то выделили первый элемент, вернули его, запомнили место где остановились. Если передана та же строка - вернули следующий элемент и опять запомнили точку остановки. Если правильно помню, именно так работает strtok - пока его не "переинициализируют" новой строкой, он возвращает указатель на следующий токен в той строке, с которой работал прошлые вызовы.
Ровно также работают выборки из БД - если объем элемента выборки, скажем, 2кБ, а в выборку попало, скажем 25млн элементов - никто не будет их все сразу в память тянуть. Будет fetch в цикле, который возвращает один (или несколько - блок - элементов) и дальше обработка того, что вернулось. Но никому не придет в голову каждый раз перебирать всю выборку с начала пока не дойдешь до нужного элемента.
Что касается контракта - он может быть неудачен как в силу неправильное изначальной постановки, так и быть наследием предыдущей реализации. И тут надо смотреть где этот контракт используется. Если локально в паре мест, то, возможно, проще поменять контракт с локальными правками и привести его к нормальному виду, соответствующему текущей реализации. Но тут надо глубже в задачу вникать, естественно.
lair
04.11.2023 20:46Но если ресурсы ограничены и нет возможности вывалить весь массив целиком, то почему бы не сделать последовательный парсинг - если передана новая строка, то выделили первый элемент, вернули его, запомнили место где остановились.
Это все равно однократный парсинг, о чем я и говорю. Просто последовательный, а не одномоментный.
aabzel Автор
04.11.2023 20:46и контракт "Также дан индекс элемента в виде положительного целого числа [...] Вернуть подстроку, которая соответствует индексу" мне не был нужен никогда
Такой API на самом деле очень удобен для модульных тестов.
lair
04.11.2023 20:46Не надо определять техническое задание по потребностям модульных тестов, особенно когда это плохо сказывается на характеристиках результата.
Не говоря о том, что "типовой" контракт ничем не хуже для тестов.
aabzel Автор
04.11.2023 20:46-3Когда код пишут без намерения его тестировать, то приводит это потом к спагетти коду и вот таким жалким оправданиям
includedlibrary
04.11.2023 20:46+2Так в чём проблема протестировать парсер, который на каждый чих не разбирает строку заново?@lair и не говорил, что код не надо тестировать. Да и аргумент слабый. Давайте посмотрим на ваш апи, который легко тестировать, как вы сказали. Ваш код получился очень сложным и с ошибками (как вам показали, возможно переполнение буфера). Получается, что дело не только в апи
lair
04.11.2023 20:46+1Я, вроде бы, не предлагал не тестировать код. Я написал, что контракт надо определять не через тесты, а через реальное использование. Тестировать его после этого все равно можно (и нужно).
iig
04.11.2023 20:46-2Спагетти-код - спагетти-тесты.. Говнокод - говнотесты ;)
Не вижу, каким образом пожелание "чтобы модульные тесты" противоречит пожеланию "решать задачу бизнеса".
BugM
04.11.2023 20:46Код пишется чтобы решить задачу бизнеса. Тесты это проблема разработки. Любой нормальный код решающий задачу бизнеса можно протестировать. Написание тестов это вообще не критерий для выбора АПИ и контрактов.
aabzel Автор
04.11.2023 20:46Можно написать код который будет решать задачу бизнеса.
При этом там будут функции по 5000 строк, магические циферки на каждой строчке, доступ в регистры в каждом файле, дублирование кода, вставка препроцессором #include *.c файлов и тому подобное.
И такой код невозможно будет покрыть тестами. Потому что он весь написан как одна мега-функцияй main().
Это так разработать электронную плату без тестовых падов.BugM
04.11.2023 20:46-1Вы точно пишете продакшен код для бизнеса? Просто вы такие странные штуки противопоставляете друг-другу что закрадываются сомнения.
Если что это перпендикулярные измерения. Они вообще никак не связаны. Нужно писать просто хороший код для решения задач бизнеса. Есть много мест где именно так и поступают. Методики тестирования уже достаточно развиты чтобы не писать продакшен код для тестирования. Можно тестировать любой нормальный код.
SpiderEkb
04.11.2023 20:46Мне всегда казалось что тесты пишутся под код, но никак не наоборот.
Все контракты все-таки разрабатываются для получения максимальной эффективности для заданного (!!!) функционала, а не "вообще на все случаи жизни" и не "чтобы тестировать проще было".
lair
04.11.2023 20:46+1Мне всегда казалось что тесты пишутся под код, но никак не наоборот.
В TDD - не так. В TDD предполагается, что тесты пишутся под требования (т.е. под предполагаемый сценарий использования), а уже потом код пишется под тесты.
Но я не говорю, что это обязательный подход, просто напоминаю, что так тоже можно, и это работает.
SpiderEkb
04.11.2023 20:46+1Ну черт его знает... Интуитивно не кажется оптимальным, но не значит что это на самом деле так.
Мне казалось, что решение реализуется для наиболее эффективной реализации сценариев его использования, а тесты - это просто имитация этих сценариев.
lair
04.11.2023 20:46Интуитивно не кажется оптимальным, но не значит что это на самом деле так.
У оптимальности много критериев. Для меня для определенных случаев TDD "оптимально", потому что я минимизирую риск написать тесты, которые находятся под влиянием решения, и поэтому не покрывают какой-то edge case. Если думать над тестами до решения, можно (а) найти противоречия в требованиях до написания кода, и (б) написать тесты, которые максимально близки к требованиям (а не к коду).
Но это все субъективно. Мне так удобнее.
includedlibrary
Я так понимаю, функция
csv_proc_fetch_valueкопирует строку из temp в out_buff. Почему бы сразу в out_buff не писать в таком случае?aabzel Автор
Да. Всё верно.
mentin
меня больше интересует - зачем копировать в
tempвсё подряд :)csv_proc_fetch_valueхотя бы копирует только нужное значение из temp, а вот в temp пишется всё подряд, любое встреченное значение, вне зависимости от строки и индекса. Потом уже, при совпадении строки и индекса - нужное значение из него будет копироваться в out_buff. Почему просто не запомнить указатель на начало значения, и избавиться от temp полностью (там еще и размер этого буфера не проверяется - легко устроить buffer overflow), я не могу понять. Единственная гипотеза - это было скопировано из настоящего парсера, где обрабатывались кавычки, и временный буфер использовался для нормализации двойных кавычек. Но врядли.