47 Атрибутов Хорошего С-кода

[GuDron]

Этот текст адресован когорте программистов на С(ях). Это не академические атрибуты из учебников это скорее правила буравчика оформления сорцов из реального prod(а). Некоторые приемы совпали с MISRA, некоторые с CERT-C. А кое-что является результатом множества итераций инспекций программ и перестроек после реальных инцидентов. В общем тут представлен обогащенный концентрат полезных практик программирования на С(ях).

*1–Все функции должны быть менее 45 строк. Так каждая функция сможет уместиться на одном экране. Это позволит легко анализировать алгоритм и управлять модульностью кода. Также множество мелких функций удобнее покрывать модульными тестами.

*2–Не допускать всяческих магических чисел в коде. Это уничтожает читаемость кода. Все константы надо определять в перечисления заглавными буквами в отдельном файле для каждого программного компонента.

*3–На все сборки должна быть одна общая кодовая база (общак, репа). Модификация в одном компоненте должна отражаться на всех сборках организации, использующих компонент (например алгоритмы CRC). Это позволит сэкономить время на создание новых проектов для новых программ.

*4–Все .с файлы должны быть оснащены одноименным .h файлом. Так эффективнее переносить, анализировать и мигрировать проекты на очередные аппаратные платформы. И сразу понятно, где следует искать прототипы функций из *.c файлов.

*5–Аппаратно-зависимый код должен быть отделен от аппаратно независимого кода по разным файлам и разным папкам. Так можно тестировать на другой архитектуре платформо-независимые функции и алгоритмы. Всякую математику, калькуляторы всяческих CRC(шек) и работу со строчками.

6--Константы следует определять при помощи перечислений enum в большей степени, чем препроцессором. Так можно собрать константы из одной темы в одном месте и они не будут разбросаны по всему проекту.

7–Не вставлять функции внутрь if() . Коды возврата приходится анализировать пошаговым отладчиком до проверки условия.

это очень плохо:

if (MmGet(ID_IPv4_ROLE, tmp, 1, &tmp_len) != MM_RET_CODE_OK) {
    return ERROR_CODE_HARDWARE_FAULT;
}

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

int ret = MmGet(ID_IPv4_ROLE, tmp, 1, &tmp_len);
if (ret != MM_RET_CODE_OK) {
    return ERROR_CODE_HARDWARE_FAULT;
}

8–Использовать static функции везде, где только можно. Это повысит модульность.

*9–Используй препроцессорный #error для предупреждения о нарушении зависимостей между компонентами.

#ifndef ADC_DRV_H
#define ADC_DRV_H

#ifdef __cplusplus
extern "C" {
#endif

#include <stdbool.h>
#include <stdint.h>

#include "adc_bsp.h"
#include "adc_types.h"

#ifndef HAS_MCU
#error "+ HAS_MCU"
#endif

#ifndef HAS_ADC
#error "+ HAS_ADC"
#endif

bool adc_init_channel(uint8_t adc_num, AdcChannel_t adc_channel);
bool adc_init(void);
bool adc_proc(void);
bool adc_channel_read(uint8_t adc_num, uint16_t adc_channel, uint32_t* code);

#ifdef __cplusplus
}
#endif

#endif /* ADC_DRV_H  */

*10--Если что-то можно проверить на этапе make файлов, то это надо проверить на этапе make файлов. Каждый компонент должен проверять, что подключены нужные зависимости. Это можно сделать через условные операторы make файлов.

$(info I2S_MK_INC=$(I2S_MK_INC))
ifneq ($(I2S_MK_INC),Y)
    I2S_MK_INC=Y

    mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST)))
    $(info Build  $(mkfile_path) )

    I2S_DIR = $(WORKSPACE_LOC)bsp/bsp_stm32f4/i2s
    #@echo $(error I2S_DIR=$(I2S_DIR))

    INCDIR += -I$(I2S_DIR)
    OPT += -DHAS_I2S

    SOURCES_C += $(I2S_DIR)/i2s_drv.c

    ifeq ($(DIAG),Y)
        SOURCES_C += $(I2S_DIR)/i2s_diag.c
    endif

    ifeq ($(CLI),Y)
        ifeq ($(I2S_COMMANDS),Y)
            OPT += -DHAS_I2S_COMMANDS
            SOURCES_C += $(I2S_DIR)/i2s_commands.c
        endif
    endif
endif

*11--Если что-то можно проверить на этапе препроцессора, то это надо проверить на этапе препроцессора. Каждый компонент должен проверять, что подключены нужные зависимости. Это можно сделать через макросы компонентов.

*12–Если что-то можно проверить на этапе компиляции, то это надо проверить на этапе компиляции (static_assert(ы)). Например можно проверить, что в конфигурациях скорость UART не равна нулю. В RunTime не должно быть проверок, которые можно произвести на этапе компиляции, препроцессора или make файлов.

*13–Каждой set функции должна быть поставлена в соответствие get функция. И наоборот. Это позволит написать модульный тест для данного параметра.

*14–Если переменная это физическая величина, то в суффиксе указывать размерность (timeout_ms). Это увеличивает понятность кода.

*15–Все Си-функции должны всегда возвращать код ошибки. Минимум тип bool или числовой код ошибки. Так можно понять, где именно что-то пошло не так. Проще говоря, не должно быть функций, которые возвращают void. Функции void это, по факту, бомбы с часовым механизмом. В один день они отработают ошибочно, а вы об этом ничего даже не узнаете.

*16–Для каждого программного компонента создавать несколько *.с *.h файлов:

Файл компонента или драйвера​	h​	c​
файл констант​	*​	​
файл типов данных​	*​	​
файл команд CLI​	*​	*​
файл энергонезависимых параметров​	*​	​
файлы конфигурации по умолчанию​	*​	*​
файлы диагностики​	*​	*​
файлы с модульными тестами​	*​	*​
файлы самого драйвера. Функционал и бизнес логика.​	*​	*​

Это позволит ориентироваться в коде и управлять модульностью.

17–Если функция получает указатель, то пусть сразу проверяет на нуль значение указателя. Так прошивки не будут падать при получении нулевых указателей. Это повысит надежность кода. Вы же не знаете как и кто этот код будет испытывать. Хорошая функция всегда проверяет то, что ей дают.

18–Если есть конечный автомат, то добавить счетчик циклов. Так можно будет проверить, что автомат вообще вертится.

19–В идеале все переменные должны иметь разные имена. Так было бы очень удобно делать поиск по grep. Но тут надо искать компромисс с наглядностью.

20–У каждой функции должен быть только 1 return. Это позволит дописать какой-то функционал в конце, зная, что он точно вызовется.

21–Не использовать операторы >, >= Вместо них использовать <, <= просто поменяв местами аргументы там, где это нужно. Это позволит интуитивно проще анализировать логику по коду. Человеку еще со времен школьной математики понятнее, когда то, что слева - то меньше, а то, что справа - то больше. Так как ось X стрелкой показывала вправо. Особенно удобно при проверке переменной на принадлежность интервалу. Получается, что > и >= это вообще два бессмысленных оператора в языке С.

*22–В проекте обязательно должны быть модульные тесты. Тесты это просто функции, которые вызывают другие функции в run-time. Это позволит сделать безболезненную перестройку кода, когда архитектура начнет скрипеть. Тесты можно вызывать как до запуска приложения, так и по команде из UART- CLI.

 

[GuDron]

*23–Избегайте бесконечных циклов while (1) при блокирующем ожидании чего-либо. Например ожидание прерывания по окончании отправки в UART. Прерывания могут и не произойти из-за сбоя. while (1) это просто капкан в программировании. Всегда должен быть предусмотрен аварийный механизм выхода по TimeOut(у) как тут.

bool UartSendWaitLl(uint8_t uart_num, uint8_t* tx_buffer, uint16_t length) {
    bool res = false;
    // We send mainly from Stack. We need wait the end of transfer.
    UartHandle_t* UartNode = UartGetNode(uart_num);
    if(UartNode && tx_buffer && length) {
         UartNode->uart_h->EVENTS_TXDRDY = 0;
         UartNode->uart_h->EVENTS_ENDTX = 0;

         UartNode->uart_h->TXD.PTR = (uint32_t)tx_buffer;
         UartNode->uart_h->TXD.MAXCNT = length;
         UartNode->uart_h->TASKS_STARTTX = 1;
         uint32_t start_ms =  time_get_ms32();
         uint32_t cur_ms = time_get_ms32();
         uint32_t diff_ms = 0;
         while(!UartNode->uart_h->EVENTS_ENDTX) {
            cur_ms = time_get_ms32();
            diff_ms = cur_ms - start_ms;
            if(UART_SEND_TIME_OUT_MS < diff_ms) {
                res = false;
                break;
            }
         }
         res = true;
    }
    return res;
}

24–Использовать макрофункции препроцессора для кодогенерации одинаковых функций или пишите кодогенераторы, если препроцессор запрещен MISRA(ой). Копипаста - причина программных ошибок №1.

*25--Все высокоуровневые функции в конец .с файла. Это избавит от нужды указывать отдельно прототипы static функций.

26–Скрывать область видимости локальных переменных по максимуму.

27–Если код не используется, то этот код не должен собираться. Это уменьшит размер артефактов. Уменьшит вероятность ошибок.

28–Если вы в С передаете что-то через указатель или возвращаете через указатель, то указываете направление движения данных приставками in, io или out.

Например:

void ProcSomeData(unsigned char* in_buffer,
                  unsigned char* out_buffer,
int len,
int *out_len);

Это позволит легче читать прототипы, не погружаясь в тело функции

29–Давайте переменным осмысленные имена, чтобы было удобно grep(ать) по кодовой базе

*30--Если в коде есть список чего-либо (прототипы функций, макросы, перечисления), то эти строки должны быть отсортированы по алфавиту. Если сложно сортировать вручную, то можно прибегнуть к помощи утилиты sort. Это позволит сделать визуальный бинарный поиск и найти нужную строчку. Также при сравнении 2-x отсортированных файлов отличия будут минимальные.

32–Функции CamelCase переменные snake_case.

*33–Все .h файлы снабжать защитой препроцессора от повторного включения. Это же касается *.mk файлов.

*34–Сборка из Makefile(ов) является предпочтительнее чем сборка из GUI-IDE. Makе позволяет по-полной управлять модульностью кодовой базы.

*35--Для синтаксического разбора регистров использовать объединения вкупе с битовыми полями.

/*Table 15. IB2-ADDR: I0000010*/
typedef union {
    uint8_t reg_val;
    struct{
        uint8_t clipping_information:1;
           uint8_t output_offset_information:1;
           uint8_t input_offset_information:1;
          uint8_t fault_information:1;
          uint8_t temperature_warning_information: 3;
          uint8_t res:1;
    };
}Fda801RegIb2Addr_t;

Это позволит делать парсинг полей одной строчкой.

Fda801RegIb2Addr_t  Reg;
    Reg.reg_val = reg_val;

*36–Соблюдать программную иерархичность. Низкоуровневый модуль не должен управлять (вызывать функции) более высокоуровневого модуля. UART не должен вызывать функции LOG. И компонент LOG не должен вызывать функции CLI. Управление должно быть направлено в сторону от более высокоуровневого компонента к более низкоуровневому компоненту. Например CLI->LOG->UART. Не наоборот.

*37–Делать автоматическое форматирование отступов исходного кода. Подойдет например бесплатная утилита clang-format или GNUIndent. Это позволит делать простые выражения при поиске по коду утилитой grep. И будет минимальный diff при сравнении истории файлов. Придерживаться какого-нибудь одного стиля форматирования. Пусть будет "единообразно безобразно".

38--При сравнении переменных с константой константу ставьте слева от оператора ==.

неправильно: if (val == 10 ) doSmth=1;

правильно: if (10 == val) doSmth=1;

Когда константа на первом месте, то компилятор выдаст ошибки присвоение к константе в случае опечатки

if (10=val ) doSmth=1;

Такая конструкция

if (val = 10 ) doSmth=1;

незаметно собирается и вызовет трагедию во время исполнения.

39–В каждом if всегда обрабатывать else вариант даже если else тривиальный. Это позволит предупредить многие осечки в программе.

40–Всегда инициализировать локальные переменные в стеке. Иначе там просто будут случайные значения, которые могут что-нибудь повредить.

*41–Тесты и код разделять на разные компоненты. То есть код и тесты должны быть в разных папках. Включаться и отключаться одной строчкой в make-файле.

*42–В хорошем С-коде в принципе не должно быть комментариев. Лучший комментарий к коду - это адекватные имена функций и переменных.

*43–Собирать артефакты как минимум двумя компиляторами (CCS + IAR) или (GCC+GHS) или (Clang+GCC) и тп. Если первый компилятор пропустил ошибку, то второй компилятор может и найти ошибку.

44–Прогонять кодовую базу через статический анализатор. Хотя бы бесплатный CppCheck. Может, найдется очередная загвоздка.

45--За if, for ... всегда должны быть { }. Весьма вероятно, что условие будет пополнено операторами.

46-- Include(ы) всегда должны только содержать только название конечного файла. Include(ы) не должны содержать часть пути к файлу.

#include "C:/Docs/code_base/trunk/utils/data_types/cyclical_buff/cyclical_buff.h"

Вот так гораздо лучше

#include "cyclical_buff.h"

Таким образом вы сможете спокойно перетасовывать файлы в папках проекта и проект по- прежнему будет собираться. И визуально это намного легче читать, поддерживать. А сами пути к заголовочным файлам надо передавать через опцию -I компилятора через make файлы. В коде же #include(ы) должны быть максимально короткими

*47-- Если вы определяете глобальную структуру, то указывайте имя полей. Так это продолжит работать, если кто-нибудь вдруг решится поменять порядок полей в структуре.

неправильно

const LedConfig_t LedConfig[LED_CNT] = {
       {LED_GREEN_ID,   1000, 0, 60, PORT_C, 13,"Green", LED_MODE_PWM, true,},
};

правильно

const LedConfig_t LedConfig[LED_CNT] = {
       {.num=LED_GREEN_ID,   
        .period_ms=1000,
        .phase_ms=0,
        .duty=60,
        .pad.port=PORT_C,
        .pad.pin=13,
        .name="Green",
        .mode=LED_MODE_PWM,
        .valid=true,},
 
};

Аномалии оформления сорцов из реальной жизни (War Stories)

1–Магические циферки на каждой строчке

2–Переиспользование глобальных переменных

3--Доступ к регистрам микроконтроллера в каждом файле проекта

4–Повторяемость кода

5--Очевидные комментарии

6–"заборы" из комментариев

7--.с файлы оснащены разноименными .h файлами.

8--Макросы маленькими буквами

9--Код без модульных тестов

10–Код как в миксере перемешанный с тестами

11--Функции от 1000 до 5000 строк и даже более

12--Вставка препроцессором #include *.c файлов.

13--Вся прошивка в одном main.c файлике 75000 строк аж подвисает текстовый редактор.

14--С-функции с именами литературных персонажей.

15--Длинные пути к файлам в #includ(ах) (начиная с корня диска С

Вывод
Общая канва такова, что надо писать С-код по таким понятиям как простота, тестопригодность, поддерживаемость, ремонтопригодность, модульность, согласованность (принцип наименьшего удивления), масштабируемость, иерархичность, конфигурируемость, изоляция компонентов и переносимость.

Если вы программируете на С(ях) микроконтроллеры, то можно еще добавить внимание на то, что надо делать UART-CLI для отладки и верификации прошивки в run-time(е), добавлять встроенные модульные тесты, собирать из самописных Makefile(ов) и всё у вас будет очень даже модульно, масштабируемо и гибко.