Этот текст адресован когорте программистов на С(ях). Это не академические атрибуты из учебников это скорее правила буравчика оформления сорцов из реального 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.

*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–Использовать макрофункции препроцессора для кодогенерации одинаковых функций или пишите кодогенераторы, если препроцессор запрещен. Копипаста - причина программных ошибок №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 ... всегда должны быть { }. Весьма вероятно, что условие будет пополнено операторами.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Если вы практикуете еще какие-то вспомогательные эффективные приемы написания программ на С(ях), то упомяните про них, пожалуйста, в комментариях.

Комментарии (130)


  1. TheCalligrapher
    27.07.2022 06:32
    +64

    Правило 1 - "короткие функции" - и правило 20 - "один return" - два антипаттерна, которые существуют лишь для поддержки друг друга.

    Нет, ни в коем случае не старайтесь следовать правила "одного return". Наоборот, при написании функции старайтесь отсечь простейшие случаи сразу, обработать их в начале функции и тут же сделать явный return. Это существенно повышает удобочитаемость кода, так как явно дает читатель возможность понять, что обработка полностью закончена и больше ничего делать не нужно (т.е. ниже по коду функции больше ничего нет). Придерживайтесь этого правила и при написании циклов: при описании итерации старайтесь отсечь простейшие случаи сразу, обработать их в начале итерации и тут же сделать явный continue.

    Стратегия "одного return" не дает понять, закончена обработка случая или нет. Чтобы побороть эту проблему сторонники "одного return" придумали правило "коротких функций". Это чушь. Функция может быть сколько угодно длинной, хоть на 100 экранов. Ничего плохого в этом нет. Функция должна реализовывать некую законченную хорошо очерченную абстракцию - это важно. А сколько экранов займет реализация это абстракции - не имеет никакого значения. Ни в коем случае не подразбивайте функции на под-функции только из соображений длины - введение притянутых за уши искусственных под-абстракций существенно затруднит чтение и понимание кода.

    12–Если что-то можно проверить на этапе компиляции, то это надо проверить на этапе компиляции (assert(ы))

    O_o? Может имелись в виду static_assert(ы)? assert(ы) - это средство проверки времени выполнения.

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

    Термином camel case называют капитализацию с в стиле camelCase.CamelCase - это не camel case.

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

    О ужас! Yoda conditions... Катастрофическая чушь, которую уже давно отправили на свалку истории. Удивило, что кто-то еще пытается их оттуда вытянуть, да еще и в рамках публикации, содержащей довольно много разумного.

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

    Еще одно фейковое правило из той же оперы, что и 38. Звучит логично, но все уже давно известно в жизни подобных проблем не возникает. Они существуют только в "рыбацких рассказов" от чайников.

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

    В таком виде - "всегда" - очень вредный антипаттерн. Занимаясь dummy-инициалиазцией локальных переменных вы лишь подавляете полезную функциональность санитайзеров кода, т.е. заметаете под ковер потенциальные ошибки.

    Инициализация - полезнейшее свойство языка, которое нужно использовать всегда, когда это возможно. А именно: когда у вас есть наготове полезное инициализирующее значение. Если такого значения у вас нет на момент объявления переменной, то лучше оставить ее неинициализированной, чем инициализировать ее "чем попало".

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

    Это - странная максима.

    Во-первых, лучший комментарий к коду - это огромное количество assert, описывающих подразумеваемые автором кода инварианты.

    Во-вторых, без текстовых комментариев не обойтись.


    1. saipr
      27.07.2022 08:03
      +1

      Во-вторых, без текстовых комментариев не обойтись.

      Да, не обойтись.


    1. pfffffffffffff
      27.07.2022 08:53
      +1

      Еще одно фейковое правило из той же оперы, что и 38. Звучит логично, но все уже давно известно в жизни подобных проблем не возникает. Они существуют только в "рыбацких рассказов" от чачайников.

      А вот тут поподробнее. Я всегда использую else на случай если в коде появится дополнительный тип константы который нужно обработать в else if


    1. randomsimplenumber
      27.07.2022 09:16
      -2

      О ужас! Yoda conditions... Катастрофическая чушь, которую уже давно отправили на свалку истории

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


      1. ReadOnlySadUser
        27.07.2022 09:37
        +1

        А зачем? Я вот люблю Yoda conditions в некоторых случаях. Удобнее читать. Например, когда из контекста сразу понятно какая переменная будет ща проверяться 3-5 раз на всякие значения, то глаз сразу целпяется за то значение, которое ожидается.


      1. TheCalligrapher
        27.07.2022 09:48
        +8

        Компиляторы уже давно "не пропускают такое", предупреждая автора предупреждением. Использование присваивания в условии обычно требует дополнительной пары скобок для подавления этого предупреждения.


    1. Vass
      27.07.2022 10:27
      +7

      Нет, ни в коем случае не старайтесь следовать правила "одного return". Наоборот, при написании функции старайтесь отсечь простейшие случаи сразу, обработать их в начале функции и тут же сделать явный return

      Множественные return хороши в C++ в чистом C, правило одного return имеет смысл, потому что перед ним как правило стоит код освобождения ресурсов и метка для goto. Соответственно везде где вы отсекаете на раннем этапе случаи вы вместо return пишите что-то типа goto free_resources_and_return, которая приведет нас к корректному освобождению памяти и к единственному return. Это, кстати, еще отличный практический пример, когда goto - не зло.


      1. IkaR49
        27.07.2022 12:12
        +2

        Немедленный return или же немедленное goto, где будет return? Кажется мне, или это контекстуальные синонимы?


        1. invasy
          27.07.2022 21:26
          +3

          Разница в освобождении ресурсов перед `return`.


          1. IkaR49
            27.07.2022 23:08

            Вы меня не поняли. Убирать за собой - хорошо. Но у меня вопрос к логике кода. И вот с точки зрения логики разницы между `if (something) return;` и `if (something) goto deinit;` совершенно ноль. Автор же предлагает писать кучу вложенных if-ов, после которых идет один return.


      1. adeshere
        27.07.2022 13:56
        +1

        Множественные return хороши в C++ в чистом C, правило одного return имеет смысл, потому что перед ним как правило стоит код освобождения ресурсов и метка для goto.

        Кстати да - в фортране тоже так делаю. Если функция должна какие-то ресурсы освобождать, то этот код пишется в конце функции, перед return, и с меткой. А по тексту вместо return пишется goto. И это почти единственный случай (имхо), когда в современном фортране goto оправдан, т.к. позволяет избежать многоуровневой вложенности, если попытаться обойтись без него.

        Правда, у такого стиля есть

        свои недостатки

        Например, у меня довольно часто функция настраивает свой контекст, потом открывает диалоговое окно и задает какие-то вопросы, потом работает с файлом и т.д. Причем брейк может из любого места случиться. Потому перед return может быть не одна метка, а две или даже три. Тут уже вопрос, а стоит ли так писать, или лучше структурно? Для себя я решаю так: пока переходы goto не "перекрещиваются", это терпимо, например:

        ...
        goto 900
        ...
        goto 800
        ....
        800   ...
        900   ...

        А вот переход goto 900 после оператора goto 800 - это уже было бы некузяво, надо функцию декомпозировать.


        1. a-tk
          27.07.2022 14:21
          +2

          Ага. А ещё такие правила рождают монстров вида:

          do {
            ...
            if (condition) break;
            ...
          } while (false);


      1. a-tk
        27.07.2022 13:57
        +2

        Так выходите по предусловиями до того, как успели захватить ресурсы. В процессе проверки инвариантов.


      1. AnthonyMikh
        27.07.2022 14:38
        +2

        Это, кстати, еще отличный практический пример, когда goto — не зло.

        Это отличный практический пример, когда в C лепят кадавров в силу убогости языка, в данном случае — из-за отсутствия RAII.


    1. adeshere
      27.07.2022 13:04
      +2

      Я здесь крокодилмимо (на С не пишу и язык не знаю), поэтому читал, примеряя все правила к своему языку. По ходу отмечал те пункты, которые к фортрану не очень подходят.... А потом открыл комментарий heCalligrapher и внезапно увидел, что почти все критикуемые им пункты (кроме 12 и 32),

      уже присутствуют в списке моих пометок

      Полностью применимы: 2-7, 13-15, 17-19, 22-23, 26-30, 35-37... Остальное (из не прокомментированного heCalligrapher), в основном, относится к специфике языка. Например, запись вида if (x = 10) ... у нас даст ошибку компиляции, поэтому писать что-то типа if (10 = x) ... никому и в голову не придет ;-) А за пп.33 отвечает язык (повторный USE игнорируется, что очень удобно при множественных ссылках на один модуль из других включаемых модулей). А вот к пп.17: небольшая добавка: если это не повлечет заметных накладных расходов, то лучше ВСЕ входные параметры функции проверять, а не только указатели. Ну и лично я стараюсь начинать файл с высокоуровневых функций, - мне проще иди от общего к частному. Если же частного слишком много, оно выноситься в отдельный модуль.

      Если убрать специфику конкретного языка, то правила "хорошего тона" будут почти одинаковы для всех языков одной группы ;-)


  1. saipr
    27.07.2022 07:19

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

    И сразу вспмнился код Minix-а, который попал мне в руки в далёком 1987 году вместе с книгой Эндрю Таненбаум «Operating Systems: Design and Implementation» (1987, ISBN 0-13-637406-9). Фактически мы только осваивали С-код. Оказалось. что код самого Minix-а читался очень легко — автор следовал этому совету 45-ти строк. Более того там встречались и пустые функции. наверное. как задел на будущее или в них отпала нужда. В этой же книге всего на дюжине страниц был описан и сам Си.


    1. edogs
      28.07.2022 01:41
      +1

      Мы до сих пор не привыкли к современной практике «читаемости» кода, когда перед функцией несколько строк ее описания для «понятности», между блоками функции достаточно часто пробелы для «читаемости», фигурные скобки блоков не только всегда присутствуют, но вынесены на отдельные строчки для «удобства» и так далее.
      В результате какой-нибудь короткий код, который в норме занимает строк 30 — растягивается на 90-120 строк, просто для «понятности и читаемости» и на один экран его уже поместить без шансов.
      Нет, мы, конечно, приучили себя писать именно так, но каждый раз это когнитивный диссонанс и насилие над собой:)


      1. saipr
        28.07.2022 06:27

        мы, конечно, приучили себя писать именно так, но каждый раз это когнитивный диссонанс и насилие над собой:)

        И в итоге, в конце видишь, что все попытки следовать этим правилам — провалились. Привести код в порядок откладываешь на потом...


  1. leotsarev
    27.07.2022 09:07
    +3

    >. Коды возврата приходится анализировать отладчиком до проверки условия.

    А может отладчик нормальный завести?


    1. adeshere
      27.07.2022 14:04
      +2

      А может отладчик нормальный завести?

      Ну вот у меня отладчик VS, и я не умею в нем поставить точку останова на вызове конкретной функции (например check_condition2() или на <Оператор_1> внутри строки:

      if (check_condition1(...) .and. check_condition2(...)) then; <Оператор_1>; else; <Оператор_2>; end if
      Поэтому для отладки мне удобнее написать:
      condition1_is_true=check_condition1(...)
      condition2_is_true=check_condition2(...)
      if (condition1_is_true .and. condition2_is_true) then
        <Оператор_1> 
      else 
        <Оператор_2>
      end if

      Может, конечно, это только в фортране такая специфика... или надо на более последнюю версию VS перейти?


      1. ReadOnlySadUser
        28.07.2022 00:28
        +1

        Чего люди только не сделают, лишь бы в EAX не заглядывать)


        1. TheCalligrapher
          28.07.2022 07:09

          Да, "заглядывание в eax" - удобнейшая практика в процессе отладки. Особенно если результат лежит не там...


  1. nixtonixto
    27.07.2022 09:18
    +1

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

    На 32-битных системах нужно ещё проверять и на максимальное значение. Например, у нас есть функция, отсчитывающая секунды за счёт выполнения 100 раз 10-миллисекундного цикла. А ей на вход по ошибке подадут 4 миллиарда.


    1. dvserg
      27.07.2022 11:12
      +1

      Описанный Вами пример не про указатели, это скорее про значения переменных.


    1. kipar
      27.07.2022 16:11
      +2

      в качестве таймаута кстати часто передают 0xFFFFFFFF (== -1), что означает "бесконечный таймаут". Если не проверять, то по сути закладывается бомба на то что "бесконечный" таймаут через приличное количество времени все-таки истечет.


  1. ob1
    27.07.2022 09:36
    +6

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

    if (val = 10 ) doSmth=1;

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

    gcc -Wall -Wextra -pedantic -Werror хотя бы по умолчанию. Хотя -pedantic по желанию.


    1. TheCalligrapher
      27.07.2022 09:56
      -6

      -pedantic обязателен, если вы пишете именно на С, а не на развеселом GCC-шном студенческом суржике.

      -pedantic-errors - лучше, но уже по желанию

      -Werror - это что-то непонятно зачем нужное.


      1. ob1
        27.07.2022 10:47
        +3

        -pedantic не всегда дружит со старым кодом. -Werror это для того, чтобы было не лень пофиксить предупреждения, или просто их не пропустить.


        1. TheCalligrapher
          28.07.2022 01:23

          Старый код - это особый случай, к теме не относящийся. Да и правильнее сказать (уж не стесняясь), что не дружит не со "старым кодом", а с безграмотным кодом.


  1. Crinax
    27.07.2022 09:51
    +3

    Спасибо за статью. Я хоть и не С-ишнтк, но вычерпнул для себя несколько правил. Правда без вопросов не остался.

    Получается, что > и >= это вообще два бессмысленных оператора языке С.

    Неужели данные операторы настолько бесполезны? Мне казалось, что мы сначала обращаем внимание на сам оператор, а потом на операнды.


    1. jmdorian
      27.07.2022 09:59
      +2

      Тоже показалось странным правило, поскольку при сравнениях с константой может приводить к yoda-стилю. Хотя далее в 38 правиле это декларируется явно, так что автор, видимо, не против. А меня от него коробит.


      1. dvserg
        27.07.2022 11:20
        +9

        Тоже не понял про > и >= . Мне думается слева лучше ставить "основную" по смыслу кода переменную, так сразу видно что нужно сравнить, а справа уже с чем сравнивать. Такой порядок анализа более естественнен .


    1. Moraiatw
      27.07.2022 10:24
      +15

      Это вообще какой то бред.

      Человеку еще со времен школьной математики понятнее когда, то что слева то меньше, а то, что справа то больше.

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


  1. cepera_ang
    27.07.2022 10:18
    +12

    Читая такие статьи понимаешь — как же хорошо не быть программистом на С. Вообще ни слова о том, чтобы что-то сделать полезное, только бесконечные правила, как бы услужить системе, которая пропустила ведь прогресс в инструментах разработки за последние тридцать лет. Причём правила ты должен отслеживать сам и если не сделаешь этого, то просто что-то будет молча не работать или взрываться во время работы.


    1. dvserg
      27.07.2022 11:22
      +3

      Не все здесь конкретно про С. Некоторые вещи про стиль и удобство, а некоторые надуманы.


      1. cepera_ang
        27.07.2022 11:27
        +2

        Что тут про стиль и удобство? Какие бы вы пункты назвали универсальными за пределами С?


        1. dvserg
          27.07.2022 11:51
          +1

          Ну к примеру

          32 - CamelCase/snake_case стиль вполне используемо за пределами C

          37- Использование автоматических форматировщиков отступов. Идея хорошая. Во многих IDE это встроенная опция.

          38 - IF, он и во многих прочих языках IF. C - подобный синтаксис много где еще используется. /хотя я с 38 не согласен/

          39 - if & else / Тоже не только про С


          1. cepera_ang
            27.07.2022 12:25
            +2

            32 — в других языках другие стили, чистой воды вкусовщина.
            37 — ок, но обычно это используется по умолчанию и даже не требует упоминания как какое-то отдельное правило, за которым нужно следить.
            38 — только в языках, которые разделяют = и == и не выдают ошибку при использовании не того в if. Rust, например, не даст присвоить, а вежливо подскажет, что ты хотел использовать ==. Даже питон так сделает.
            39 — сомнительный совет в принципе.


            1. dvserg
              27.07.2022 12:28
              +3

              Но тем не менее, оно не только про "Читая такие статьи понимаешь — как же хорошо не быть программистом на С. " ?


              1. cepera_ang
                27.07.2022 12:45
                +4

                Всё ещё считаю, что на основании статьи, в которой 91% советов касается С и 9% сомнительных правил, которые с натяжкой можно применить к другим языкам можно делать вывод «как же хорошо не быть программистом на С» :)


            1. adeshere
              28.07.2022 00:49

              del


        1. adeshere
          28.07.2022 00:50
          +1

          Какие бы вы пункты назвали универсальными за пределами С?

          Ничего не могу сказать про универсальность, но, к примеру, в фортране вполне применимы 2-7, 13-15, 17-19, 22-23, 26-30, 35-37. Язык, конечно, не самый популярный сейчас, но в своей нише весьма конкурентоспособный


          1. aabzel Автор
            29.07.2022 00:04

            Что сейчас пишут на Fortarn(е)?


            1. adeshere
              29.07.2022 00:35
              +1

              В той сфере, где я работаю - пишут программы для научных расчетов. Причем не только "одноразовки" для личного пользования (когда выбор языка определяется преимущественно тем, на чем учили программировать в ВУЗе), но и "перемалыватели чисел", так как по производительности на таких задачах современный фортран вполне себе держится в лидерах, а по простоте и удобству кодирования именно вычислений многих соперников превосходит. Насколько я знаю от коллег, во многих научных организациях (и не только в РФ) это до сих пор либо выбор номер 1, либо близко к тому.

              Кстати, вот тут я недавно написал небольшое эссе о том, как и зачем научные сотрудники пишут "одноразовые" программы (носителям чересчур серьезного выражения лица просьба ссылку не открывать, не рискуйте, хотя в каждой шутке есть доля шутки ;-).


    1. Albert2009ru
      27.07.2022 12:35
      +3

      Практически все эти правила, MISRA уж точно, "написаны кровью", когда корявый нечитаемый код вешает МК в какой-нибудь индустриальной системе или автомобиле. Так что в эмбеде строгое соблюдение правил очень важно.


      1. cepera_ang
        27.07.2022 12:48
        +3

        То есть вы имеете в виду: когда корявый нечитаемый код на С вешает МК и что в эмбеде при разработке на С строго соблюдение подобных правил очень важно?


        Ну, серьезно, эти правила — исключительно от недостатков самого С и раньше не было альтернатив С в некоторых областях разработки (ембед и т.д), что сделало разработку на С практически синонимом разработке в этих областях и соответственно, проблемы и недостатки С стали восприниматься как часть предметной области, хотя это вовсе не так.


        1. Albert2009ru
          27.07.2022 13:15

          Ну дайте мне пример на другом языке, где я смогу в обработчик прерывания засунуть функции обработки математики и, допустим записи в еепром, потому что мне так захотелось. И это пашет где-то на реальной системе и не сбоит.

          Очень удивлюсь. По мне так лучше элементарные правила соблюдать.


          1. cepera_ang
            27.07.2022 13:25
            +2

            Rust подойдёт?


            1. Albert2009ru
              27.07.2022 13:32
              -1

              Да хоть Java Script ;)


              1. cepera_ang
                27.07.2022 13:38
                +1

                Я думал вам нужен пример языка, отвечающего вашим требованиям :)


                Вам нужен пример кода такой? Искать конкретный пример, в котором всё именно так, как вам захотелось — не буду, но по отдельности все эти вещи есть на Rust’e. И запись в еепром и обработка математики и обработчики прерываний и код, работающий в реальных системах.


                1. Albert2009ru
                  27.07.2022 13:45
                  -1

                  Я где-то утверждал, что в расте чего-то нет???


                  1. cepera_ang
                    27.07.2022 14:28
                    +5

                    Вы утверждали, что обсуждаемые правила — это не специфика С, а специфика «области» разработки, а когда я привёл пример языка, где эта специфика будет отсутствовать, не смотря на то, что область останется той же, вы почему-то упомянули Java Script и с этого момента наш разговор выглядит как какое-то недопонимание.


                    1. Albert2009ru
                      27.07.2022 14:37
                      -2

                      JavaScript = "мне без разницы на каком языке Вы приведёте пример кода..." Не хотите соблюдать правила, не соблюдайте...


                1. kipar
                  27.07.2022 15:02
                  +4

                  Я думаю речь о том, что в обработчике прерывания не стоит делать ни сложной математики, ни тем более записи в еепром, т.к. это будет означать что другие прерывания в это время не будут обрабатываться (или будут, но только более высокого приоритета, что тоже некруто). Поэтому правило в том чтобы в прерывании только выставлять флаги, а тяжелую обработку делать в основном цикле\задачах rtos и оно не зависит от языка - хоть раст хоть js.

                  Хотя конкретно в этом посте такого правила нет, так что диалог действительно похож на недопонимание.


                  1. Albert2009ru
                    27.07.2022 15:09
                    +1

                    Спасибо. Вот не умею я доходчиво и развёрнуто донести до собеседника свои мысли. Беда у меня с этим ????????????


                  1. aabzel Автор
                    27.07.2022 15:17

                    Есть такие понятия как верхняя и нижняя половины обработчика прерываний.

                    Верхняя половина работает в ISR ставит флаги

                    Нижняя половина работает в супер цикле и запускает долгие функции обработчики.


                    1. nixtonixto
                      27.07.2022 16:37

                      Тогда проще написать машину состояний или вообще RTOS, чем плодить циклы и суперциклы и всё это на проце с вытесняющими прерываниями.


                      1. kipar
                        27.07.2022 16:39
                        +2

                        Написали же. Верхние и нижние обработчики это термин из ядра Linux.


    1. rtemchenko
      28.07.2022 01:08
      +1

      Многие правила очень даже подходят для других языков.

      Ну и инструменты разработки не панацея. Никакой инструмент разработки не поможет, когда код выглядит как елка с функциями на 1к строк.


      1. cepera_ang
        28.07.2022 08:25

        Поможет pre-commit hook в гите, проверяющий на цикломатическую сложность :) Инструмент разработки? Инструмент. Поможет? Поможет :)


        1. rtemchenko
          28.07.2022 09:10

          Ну дык без правил этот инструмент ничего не сделает.

          Вот автор в статье дает рекоммендации по правилам.

          Как именно правила енфорсить - это уже за рамками изначальной статьи.

          Варианты есть разные:

          • Гит хук

          • Статический анализатор в ИДЕ

          • Серверный линтер

          • ПР ревью для сложных правил

          • ???


  1. Moraiatw
    27.07.2022 10:18
    +5

    1–Все функции должны быть менее 45 строк.

    Интересно, как разбивать многовариантный switch по 45 строк...


    1. Kyoki
      27.07.2022 11:41
      +6

      Ну как как...

      switch(i){
        case 1: ... break;
        ...
        case 39:... break;
        default: funcSwitch4050(...); break;
      }


      1. Moraiatw
        27.07.2022 16:40
        -1

        точно ))


    1. eimrine
      27.07.2022 12:58
      -9

      switch это не функция, а макрос.


    1. aabzel Автор
      28.07.2022 23:37
      -1

      Когда Switch разрастается больше 45 строк, то надо делать статические LookUpTable(лы) (LUTы). Элементом LUT(а) может являться указатель на функцию до 45 строк.

      static const AdcChannelInfo_t AdcChannelInfoLut[] = {
          {.code = ADC_CHANNEL_0, .adc_channel = ADC_CHAN_0},  
          {.code = ADC_CHANNEL_1, .adc_channel = ADC_CHAN_1},
             ....
          {.code = ADC_CHANNEL_999, .adc_channel = ADC_CHAN_999},
      };
      
      uint32_t AdcChannel2HalChan(AdcChannel_t adc_channel) {
          uint32_t code = 0;
          uint32_t i = 0;
          for(i = 0; i < ARRAY_SIZE(AdcChannelInfoLut); i++) {
              if(AdcChannelInfoLut[i].adc_channel == adc_channel) {
                  code = AdcChannelInfoLut[i].code;
                  break;
              }
          }
          return code;
      }
      


      1. cepera_ang
        28.07.2022 23:39

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


        1. aabzel Автор
          28.07.2022 23:45

          Лучше потому что функции помещаются на один экран.
          А для поиска по коду можно обратиться к утилите grep.


      1. kozlyuk
        29.07.2022 00:26
        +2

        Ради того, чтобы соблюсти формальное правило, вы мешаете компилятору оптимизировать switch в бинарный поиск, например. Здесь количество сравнений зависит от данных и в среднем больше, чем необходимо для 45 вариантов.


  1. MaxMxMz
    27.07.2022 11:17
    +11

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


  1. serge-sb
    27.07.2022 11:42
    +1

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

    Сомнительно. Если, к примеру, функция не вызывается, то её в итоговом бинарнике не будет.


    1. screwer
      27.07.2022 11:55
      +1

      Только в тривиальных случаях или если у вас работает LTCG. Иначе линкер притащит объектный файл целиком. Попробуйте сделать статическую либу и вызвать единственную функцию из обьектника, входящего в нее.

      Ещё одна мера противодействия - размещение каждой функции в выделенной ей секции. Тогда линкер сумеет эти секции убрать.


      1. serge-sb
        27.07.2022 12:55
        +1

        Точно, -ffunction-sections.


  1. RekGRpth
    27.07.2022 11:52
    +2

    на эту тему была книга "Правила программирования на Си и Си++. Ален И. Голуб"


    1. Rio
      29.07.2022 08:59

      Одна из лучших, если не самая лучшая, книга о программировании из всех, что я прочитал. Не только о Си и плюсах, там много очень глобальных вещей, в том числе о подходе к разработке. Никакой воды, всё из практики, всё с объяснением почему и как. Недавно перечитывал, и в очередной раз поражался, насколько вещи, там описанные, актуальны до сих пор. Большинство граблей, на которые наступают коллеги по работе, там расписаны. Так что пытаюсь по мере сил рекламировать в коллективе сию книжку.


  1. chnav
    27.07.2022 12:04
    +9

    В стародавние времена "подпрограмму" создавали в тех случаях, когда код используется более одного раза. Сейчас же всё перевернули с ног на голову - давайте разобьём код на стопицот функций, которые затем вызываются ровно в одном месте и нигде более. В итоге разбухшие заголовочные файлы либо декларации функций в начале C-файла, т.е. функция объявляется и описывается кодом в двух разных местах. А ведь ещё надо придумать несущие смысл названия, названия которых вылетают из головы как только исчезли с экрана после скроллинга. И тут копипаста - наше всё. Через некоторое время начинаешь рыскать по коду в надежде понять, что же эта функция в 45 строк реально делает. А если ещё создавать документацию в DoxyGen... Переизбыток функций не добавляет понимания.

    В своем коде для личного пользования я не стесняюсь писать длинные функции и, тем более, развёрнутые комментарии перед функцией, в описаниях переменных, перед каждым блоком кода, иногда чуть ли не в каждой строке.


    1. yeswell
      27.07.2022 20:41
      +3

      Но ведь цель функции -- это не только переиспользование кода

      Она выполняет ещё одну важнейшую вещь: даёт осмысленное название куску кода

      Что позволяет лучше понимать код без комментариев =)


    1. volodyaleo
      28.07.2022 12:53
      +1

      Если у функции название адекватное и написана нормально, то не надо ничего рыскать чтобы понять что функция делает. А босс-функции невлезающие в экран разбирать иногда неделями надо. Когда только автор функции знает как она работает - это кошмар для коллег.

      Разбухшие заголовочные файлы говорят о ужасной разбивке кода на модули.

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

      К тому же всё в одной функции делать зачастую бессмысленно. Либо компилятор сам заинлайнит либо можно заставить.


      1. a-tk
        28.07.2022 13:29
        +2

        Комментарии нужны там, где надо объяснить, почему функция работает так или иначе, дать ссылки на документацию (например, ссылка на раздел или выдержка из даташита) и т.п.


    1. TheCalligrapher
      28.07.2022 19:12
      +3

      Нет, такого простого критерия, как "код используется более одного раза", никогда не существовало, кроме как у нерадивых рукопопов.

      Разбиение кода на функции по любому формально-механическому принципу - это грубый, кривой и косой антипаттерн, который только сделает код нечитаемым. К этой категории относится и "код используется более одного раза", и "код содержит слишком много строк" и прочие подобные механические критерии.

      Разбиение кода на функции всегда делается исключительно по признаку выделения естественных, хорошо очерченных параметризованных абстракций. Функция должна делать что-то одно, что-то понятное, что-то легко описываемые в нескольких словах. При этом функция должна быть более самостоятельна, чем вызывающий ее код, то есть она должна обладать дополнительной ценностью - быть потенциально полезной и в других местах кода, а не только в тех в которых она вызывалась в момент своего создания. Это - важный критерий того, что функция имеет право на существование. Также хорошим признаком правильно выделенной функции является то, что для такой функции легко придумать название.

      Если в вашем коде не получается выделить подобные участки, из которых можно вылепить такие аккуратные, обособленные, самостоятельные и потенциально переиспользуемые абстракции, то не нужно разбивать его на функции, даже если он представляет из себя "стену текста" на 100500 строк и даже если в нем есть повторения.


      1. chnav
        28.07.2022 19:33
        +1

        Если в вашем коде не получается выделить подобные участки, из которых можно вылепить такие аккуратные, обособленные, самостоятельные и потенциально переиспользуемые абстракции, то не нужно разбивать его на функции, даже если он представляет из себя "стену текста" на 100500 строк и даже если в нем есть повторения.

        Ну я примерно это и имел в виду, если говорить про современные языки программирования и C в частности - если участок кода потенциально может использоваться в другом месте то можно и выделить в отдельную функцию, тут на усмотрение программиста.

        Что касается моего первого тезиса (код используется более одного раза) это отсылка к истории 70-90-х про медленные процессоры, фортран, ассемблер и всё такое.

        PS: минусанул точно не я


        1. viordash
          28.07.2022 22:02
          +1

          Выделение одинаковых кусков кода в отдельную функцию это не только экономия пространства, но, как по мне более значащий момент, это то что фикс в этой функции, распространится по всему коду.

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


          1. TheCalligrapher
            28.07.2022 22:13
            +1

            А кто гарантирует, что он будет действительно фиксом для "всего кода"?

            Если выделение функций делалось механически "по повторяющемуся коду", а не путем продуманного выделения абстракций, то вполне запросто может получится, что "фикс в этой функции" окажется фиксом для одного контекста ее вызова, но багом для других контекстов ее вызова.

            Когда эта ситуация обнаруживается, как правило начинается дополнительный "фиксинг" путем введения "странных параметров" для данной функции. Это параметры, которые не несут никакой осмысленной абстрактной семантики, а служат лишь для идентификации места, из которого вызвана функция. Им, как правило, дают какие-то ничего не говорящие странные имена, вроде mode, passили for_johnny.

            А в реальности, на самом деле, просто не надо было выносить этот, пусть даже немного повторяющийся код в отдельную функцию.


          1. a-tk
            28.07.2022 22:17
            -1

            Есть две принципиально разные ситуации повторения кода.

            1. Код повторяется случайно - повторяющийся или близкий код с высокой вероятностью заживёт параллельными ветками при эволюции каждой из них.

            2. Код повторяется системно - эволюция будет только синхронно.

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


            1. viordash
              28.07.2022 23:17
              +1

              ключевое слово, "одинаковый",

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


              1. a-tk
                29.07.2022 08:06
                +1

                Речь не про идентичный код, ведущий себя по-разному. Код может в течение своего жизненного цикла претерпевать изменения. И два кусочка одинакового кода - кандидата на выделение функции могут эволюционировать независимо.

                Немного синтетическая иллюстрация:

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

                Вначале казалось, что это общая операция, но потом выяснилось, что показалось и эволюционные треки одного и того же кода разошлись.


                1. viordash
                  29.07.2022 11:02

                  я вас понимаю и согласен с вашим примером, такое случается.

                  Но в этом случае уже код не идентичен и это уже совершено другая функция.


                  1. a-tk
                    29.07.2022 11:14

                    О том и речь: код, который изначально выглядел одинаково и мог казаться хорошим кандидатом на выделение в функцию, через какое-то время таким перестал являться. То есть имело место случайное совпадение на начальном этапе.


  1. zvszvs
    27.07.2022 12:19
    +4

    Из области не правил, но оптимизации. Если есть цикл от 0 до N и не важно в каком направлении его выполнять, то лучше двигаться не от 0, а к 0, т.к. на многих архитектурах сравнение с константой и сравнение с 0 - сильно различны по размеру команды и/или скорости выполнения (при условии возможности оптимизации компилятора). Например для x86 (32 бита):

    cmp eax,5

    и

    test eax,eax
    Часто второе оптимальнее даже для аппаратно независимых языков типа Java.


    1. cepera_ang
      27.07.2022 12:28

      А есть бенчмарк, а то выглядит как вредный совет, ради призрачной выгоды (процессор всё равно закопает эту проверку глубине спекулятивного исполнения и для него важнее, чтобы переход предсказывался хорошо и тело цикла в кеши помещалось).


      1. chnav
        27.07.2022 13:41
        +2

        Какие тут бенчмарки ))) Большой экономии по размеру не будет, т.к. в цикле 0 -> 5 идёт сравнение с константой "5" (для хранения которой 4 байта на константу), зато инициализация будет xor eax,eax.

        В цикле 5 -> 0 эта же константа 5 (4 байта) переместится в инициализацию перед циклом, но экономия на сравнении с нулем. Как показано в комментарии ниже, этот случай действительно может быть оптимизирован за счет конструкции dec eax, jz label, , но там выигрыш будет 1-2 байта в размере.

        Это скорее для старых процессоров или спортивного программирования.


        1. cepera_ang
          27.07.2022 14:29
          +5

          А потом компилятор анроллит цикл десять раз и вся экономия одного байта пойдёт прахом.


    1. dmitryrf
      27.07.2022 12:35
      +1

      Тогда скорее
      dec eax
      jz label


      Но компилятор может найти и более интересные варианты.


      1. a-tk
        27.07.2022 14:03
        +1

        loop/loopz/loopnz забыли... И не зря: она медленнее чем несколько инструкций проверки/перехода.


    1. Miiko
      28.07.2022 07:08
      +1

      Согласен. Тем более, что в Си для этого есть специальный оператор "goes toward":

      int i = N;
      while (i --> 0) { ... }

      Цикл будет выполнен для i от N-1 до 0, всего N раз


      1. aabzel Автор
        29.07.2022 00:02
        +1

        Никакой это не специальный оператор.
        После автоматического форматирования получится 2 оператора (--, пробел, >).
        while (i-- > 0) { ... }


    1. TheCalligrapher
      28.07.2022 08:45
      +3

      В ситуациях, когда счетчик цикла не участвует в теле цикла, любой уважающий себя компилятор запросто выполнит такую оптимизацию за вас.

      В ситуациях, когда счетчик цикла участвует в теле цикла, такая оптимизация навскидку не применима.

      Не надо пытаться в голове переводить С код в машинный код. В 9 случаях из 10 ваши мысленные переводы будут мало коррелировать с реальностью. С - не ассемблер.


    1. DrGluck07
      28.07.2022 14:21
      -1

      А разве компиляторы уже давно не делают это сами при подходящих обстоятельствах?


  1. dmitryrf
    27.07.2022 12:31
    +2

    В целом список полезный, хотя, как уже многие написали выше, некоторые правила появились от несовершенства старых компиляторов и без них можно обойтись, улучшив читаемость кода. Очень рекомендую включить как можно больше всяких warnings и werror, чтобы отловить максимум ошибок и тонких мест на этапе компиляции. Вот хорошая статья по теме: interrupt.memfault.com/blog/best-and-worst-gcc-clang-compiler-flags


  1. kipar
    27.07.2022 13:52
    +2

    Согласен с большинством пунктов: 1,2,4, 5,8,9,11,12,14,16,23,24,26,27,33,35,37,41,44.

    Есть пункты которые выглядят нейтрально - просто кодестайл, можно придерживаться их, а можно использовать другой кодестайл, не хуже.

    Те с которыми не могу согласиться в том виде в каком они сформулированы:

    (6) - Enum для перечислений. Если константа - "номер протокола USB", то для нее enum логичен, если "максимальное число элементов в списке" - нет.

    (7) и (20) по-моему противоречат друг другу. Если у нас один ретурн то нет никакого вреда от функций в условии, ведь мы можем зайти в нее и поставить точку останова на этот return. при этом оба будут раздувать код и из тривиальной функции на три строчки могут сделать монстра на полэкрана.

    (15) опять же, при буквальном соблюдении вместо

    void exec_task_dac(void * argument)
    {
      init_dac();
      while(1)
      {
      	process_dac();
        osDelay(1);
      }
    }

    будет монстр на полэкрана который проверяет какие-то там коды возврата, плюс резервировать под них коды ошибок

    (19), (20), (29) - в IDE есть Find Usages, которая правильно находит все вхождения даже если переменные называются одинаково. Поэтому это в определенных случаях удобно, но в атрибут хорошего кода я бы возводить не стал.

    (21) - сразу нет. if(get_count() > MAX_COUNT - RESERVE) сразу понятно что элементов слишком много, if(MAX_COUNT - RESERVE < get_count()) требует в уме переворачивать условие. Как кодестайл в конкретном проекте может и ок, но читаемости скорее мешает.

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

    (30) - енумы должны быть отсортированы по значению, в остальных случаях - сгруппированы по категориям (поиск категория-функция эффективнее алфавитного если не помнить точного названия функции).

    (38) - к счастью, gcc ругается на if( a = something), так что не вижу смысла уродовать код.

    (39) - часто использую двухстрочные условия типа

    if(no_action_required)
      return;

    ни вижу смысла раздувать эти проверки из-за каких-то возможных багов - больше проблем принесет если эти проверки начнут занимать на экране больше места чем сама логика.

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


  1. huder
    27.07.2022 14:36
    +2

    15… Проще говоря, не должно быть функций, которые возвращают void
    32. Функции CamelCase переменные snake_case

    void proc_some_data(unsigned char* inBuffer, ....)


    1. cepera_ang
      27.07.2022 14:39
      +1

      Правила становятся просто благими пожеланиями, если нет механизма их энфорсить. А С ценят за свободу, поэтому такие механизмы и не предусмотрены.


  1. CoolCmd
    27.07.2022 16:21
    +1

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

    у меня гибридный подход. если размер функции небольшой + логика несложная + вложенность небольшая, то return один. иначе ставлю несколько return, чтобы упростить написание и чтение функции. заодно позволяет не разбивать функцию на 100500 частей (правило 1), замусоривая исходники (в первую очередь folding) и тратя силы на придумывание осмысленного названия для функций.


  1. DeepFakescovery
    27.07.2022 16:55
    +4

    Берегитесь языков, для которых пишут многотомные правила написания на них.


    1. Miiko
      28.07.2022 07:11
      +1

      Из приведенных правил очень немногие специфичны для Си, большинство применимо (практически) к любому языку


  1. Rio
    27.07.2022 20:05
    +6

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

    В целом всё так, но про "все константы" это наверное зря. Я не раз встречал код, авторы которого слишком буквально понимали правило "не допускать чисел в коде". Константами там было задано вообще всё, включая 1 и 0. Это был код а-ля

    x = (y >> TEN) & ONE;

    Это хуже читается, искать баги становится сложнее, ведь каждую такую константу нужно проверять, а на самом ли деле ONE равно 1, а вдруг автор совсем не то имел в виду.

    Ещё один пример - когда константами забиваются значения битов/полей в регистре. Да, это отличное правило, и в общем случае ему очень желательно следовать. Но, опять-таки, на практике встречается код с огромным количеством таких констант в h-файле, скажем, драйвера, а в его си-файле по факту в паре регистров пара битов проверяется, и всё. Когда осознаёшь, что кода могло бы быть в разы меньше, а сам он был бы сильно проще и понятнее, категоричность суждений снижается, и понимаешь, что правила правилами, но в отдельных случаях правильнее, как ни странно, может оказаться от них отступить. И проверить нужный бит явно, в комментарии указав, что именно мы тут делаем и в каком пункте даташита об этом говорится. Работа поддерживающего программиста станет проще в разы.

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

    Крайне спорно. Когда работаешь в рамках одной платформы - да, удобно. Но у битовых полей большое количество подводных камней.

    Когда пишешь что-то (потенциально) мультиплатформенное, или, не дай бог, протокол обмена, то битовые поля - это боль. Сталкивался со случаем, когда в протоколе обмена битовым полем была описана большая структура. И всё ожидаемо поломалось, когда в какой-то момент обмениваться пришлось с новой системой, с другим порядком байтов. В тот раз несколько дней потратили на ручной "переворот" битовых полей. И теперь в том протоколе две структуры - одна для BE, другая для LE, что, конечно же, не очень здорово из-за дублирования кода.

    Ещё эпичный случай. Искали трудноуловимый баг в коде. Нашли, что неверно пишется значение в поле структуры. Было там примерно следующее:

    state.result = SOME_ERROR_CODE;

    При этом, SOME_ERROR_CODE равно двум, поле bar имеет тип int, но в результате записи там почему-то оказывается ноль. Почему? А потому что автор объявил поле так:

    int result: 1;

    В тот момент ему хватало двух значений для кода результата, но в какой-то момент ему понадобились ещё коды, он их добавил, а битов для поля не добавил. С точки зрения языка, тип поля - int, IDE показывает, что тип поля - int, число 3 в int влезает, компилятор не ругается. Так этот баг в итоге и остался на какое-то время незамеченным.

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

    Вот это прям очень хорошее правило. Очень-очень. Постоянно по жизни сталкиваюсь с параметрами функций и полями, где передаётся время, а размерность нигде не указана. И комментарий даже есть, но там написано - "время того-то того-то", а про единицы - ни слова. Правильно названные сущности могут уменьшить необходимость в комментариях до минимума, это верно.

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

    А вот тут я снова не согласен. Не надо так делать. Если вы пишете ответственный код, то функция, возвращающая код ошибки, обязывает вас его всегда проверять. Если автор библиотеки или драйвера сделал все свои функции возвращающими значение, даже когда это не нужно, то юзер вашей библиотеки будет либо игнорировать коды возврата (возможно, формально нарушая при этом конкретные стандарты кодирования) либо действительно проверять, даже когда это не нужно, что сильно увеличит размер кода и сильно снизит его читаемость из-за кучи ненужных if-ов и return-ов.

    Вот вы, например. часто проверяете, что там printf вернул? Многие и не знают даже, что он что-то возвращает. Таких примеров множество.

    Представим, что мы написали библиотеку для растровой графики. Там есть функция gfxInit(). Должна ли она возвращать код результата? Скорее всего да, должна, ведь инициализация может пройти неудачно, скажем, из-за нехватки памяти, из-за аппаратных причин (железо нужное на шине не нашли), и т.п. В этом случае, очевидно, ПО должно как-то отреагировать на критический сбой. А должны ли возвращать ошибку функции навроде gfxPutPixel() или gfxDrawLine()? Моё мнение - нет. Не нужно заставлять программиста, использующего ваш интерфейс, засорять свой чистый и понятный код отрисовки графики бесполезными проверками. Код станет нечитаемым адищем, а пользы для дела - ноль. Гораздо полезнее в подобных случаях (когда подсистема/библиотека может "отвалиться" в процессе работы) создать отдельную функцию для получения текущего состояния - и вот её уже можно будет вызывать периодически (один раз за такт в главном цикле работы, например), чтобы узнать, всё ли там в порядке.


    1. a-tk
      29.07.2022 21:28
      +1

      О, есть прикол с битовыми полями.... Злобный.

      struct { int field : 1; } v;
      v.field = 1;
      assert(v.field == 1); // FAIL

      Кто догадался в чём дело?


      1. TheCalligrapher
        29.07.2022 21:55
        +1

        "Догадался"?

        Во-первых, пример заезжен и ответ очевиден - битовое поле оказалось знаковым, то есть получило значение -1.

        Во-вторых, в языке С (в отличие от С++) тип int не является синонимом signed int в таком контексте. Битовое поле может оказаться знаковым, а может и беззнаковым. Это определяется реализацией. То есть в зависимости от implementation-defined факторов данный assert может выстрелить, а может и не выстрелить.


  1. alphikk
    28.07.2022 15:51
    +1

    Вот бы и для C++ такой набор советов.


    1. AnthonyMikh
      28.07.2022 16:13

      Список советов:


      1. Не пишите на C++.


      1. a-tk
        28.07.2022 22:12

        Вы хотели сказать "0. Не пишите на С++, пишите на Rust"?


        1. AnthonyMikh
          29.07.2022 01:59
          -1

          Не хочу, мне комитет Rust evangelism strike force задерживает выплаты.


      1. aabzel Автор
        28.07.2022 23:27
        -1

        Согласен. С++ не подходит для программирования MК так как С++ запрещает стандартом брать адрес функции main. А это необходимо для run-time проверки, что первичный, вторичный загрузчики и многоядерные сборки Generic(ов) легли в нужные адреса NorFlash(а).


        1. TheCalligrapher
          28.07.2022 23:40

          С++ прекрасно подходит для программирования МК.

          Программирование MK, как впрочем и любое программирование, нацеленное на конкретный экземпляр или класс хардвера, всегда и везде будет делаться с существенным привлечением нестандартных/надстандартных гарантий и возможностей, предоставляемых конкретным компилятором. Цитировать запреты стандарта при это смысла нет. Если конкретная реализация С++ позволяет вам брать адрес main и вам это действительно нужно - берите его. Это в одинаковой мере относится и к С, и к С++, и к любому другому ЯВУ.

          В частности, с педантичной точки зрения фразы типа "Линукс написан С" являются бессмысленными, ибо на стандартном С невозможно реализовать ОС. Все прекрасно понимают, что в такой реализации будут использоваться нестандартные/расширенные возможности конкретного компилятора. Это же в равной мере относится и к программированию МК.


        1. AnthonyMikh
          29.07.2022 02:01
          +2

          А вам не кажется, что в рантайме уже немного поздно проверять, что загрузчики легли в память по нужным адресам?


  1. DungeonLords
    29.07.2022 00:19
    +1

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


    1. aabzel Автор
      29.07.2022 00:32
      -1

      Американский профессор Jacob Sorber, доказывает, что люди, которые привыкли программировать в IDE имеют тенденцию становиться очень слабыми программистами.

      https://www.youtube.com/watch?v=Pqf6H1WSbeY

      потому, что User(ы) IDE не понимают какой путь проходят исходники с момента написания до попадания в Flash. За них всё делает IDE.


      1. cepera_ang
        29.07.2022 00:59
        +4

        А зимой не надо одеваться и отапливать жильё — это делает жителей холодных регионов слабыми и уязвимыми.


        Программистский мазохизм какой-то.


        Edit: это не считая того, что профессор сказал совсем не это и он не доказывает, а это просто его мнение.


      1. Rio
        29.07.2022 09:26
        +1

        Есть такое. Постоянно сталкиваюсь с тем, что новички, да и не только новички, к сожалению, не понимают разницы компиляции и линковки. Часто бывает, что видя при сборке проекта ошибку "unresolved reference", не могут понять, что это не ошибка компиляции, а просто библиотеку не подключили нужную. Или в имени функции при вызове ошиблись. В итоге, тратят на решение подобных, казалось бы, простых проблем, слишком много времени, зачастую совершая при этом какие-то рандомные действия. От IDE отказываться никто не призывает, наоборот, фичами IDE нужно пользоваться во всю, если они облегчают рутинную работу, но всё-таки нужно же знать, как работает твой инструмент. И было бы здорово, если бы обучение языку начиналось именно с этого - сначала пишем файл исходника и вручную вызываем для его сборки компилятор, затем добавляем ещё файлов и учимся делать простой мейк-файл, а уже затем переходим в IDE. Тогда и действия IDE уже не будут восприниматься как сложная и непонятная магия. Наверное.


        1. aabzel Автор
          29.07.2022 14:31

          Если вы собираете сорцы из Makefile, то вы можете инициировать сборки прямо из командной строки. А это значит, что процесс сборки можно автоматизировать. Прикреплять в Jenkins. А утром контролировать результаты сотен сборок, что отработали ночью. Производительность работы с Makefile выше.

          В случае с IDE вам придется вручную водить мышку, чтобы стартонуть сборку.


          1. a-tk
            29.07.2022 14:58

            А что мешает иметь процесс сборки, который запускается и в IDE, и вне её?


          1. TheCalligrapher
            29.07.2022 20:20
            +1

            В MSVC никто вам не мешает сразу инициировать процесс сборки классического MSVC-шного проекта из командной строки. Никакой мышкой водить никуда не надо.

            Это даже не говоря о том, что доминирование Windows и MS Visual Studio, как инструмента повседневной разработки для всех целевых платформ привело к тому, что средства автоматической конвертации проектов Visual Studio в makefile или любой другой целевой формат доступны в огромном количестве. Стандартный flow для прикладного ПО под Linux: повседневная разработка в MSVS под Windows -> автоматическая конвертация vcxproj в makefile -> сборка и тестирование под Linux.


            1. aabzel Автор
              29.07.2022 20:23
              -1

              В MakeFile нет xml для каждого проекта. Как в IDE.
              Это значит, что добавление и исключение сотен .с файлов для сотен сборок можно делать одной строчкой в make.


              1. TheCalligrapher
                29.07.2022 20:48

                Ну тут можно еще добавить такие операции, как перетасовку содержимого файлов в случайном порядке, архивирование сотен файлов и посылку их на FTP, автоматическую массовую замену идентификаторов на имена персонажей из LotR, проигрывание MP3 музыки из файлов проекта с соответствующим расширением и еще бесчисленное множество разнообразных действий, которые никому на самом деле не нужны за пределами статистической погрешности.

                И это при том, что "добавление и исключение сотен .с файлов" не представляет никакой сложности и в IDE. 


        1. aabzel Автор
          29.07.2022 14:36

          При сборке из Make можно одной строчкой включать/исключать из сотен сборок сотни файлов.

          В случае с IDE пришлось бы неделю редактировать *.xml(ки) *.ewp, чтобы сделать что-то подобное. Причем если ошибешься с пробелом или запятой в этих IDE(шных) xml, то проект вообще покорраптится и даже не откроется.

          IDE это для школоты и протопитирования, когда 1 конфигурация и 1 сборка.



          1. a-tk
            29.07.2022 14:58
            +1

            Вы сейчас про какую именно IDE говорите?


            1. aabzel Автор
              29.07.2022 15:00

              IAR


          1. TheCalligrapher
            29.07.2022 20:26
            -1

            O_o. Все повседневная разработка прикладного ПО ведется именно в IDE, ибо именно такой мгновенной интегрированной доступ к средствам сборки и отладки абсолютно необходим для эффективной разработки. IDE и родились из требований разработчиков. Повседневная разработка без IDE сегодня - это упрямое пожирание кактуса.

            Именно из-за того, что MS Visual Studio с гигантским отрывом является самой мощной, удобной и эффективной IDE на сегодняшний день, и сложилась нынешняя парадоксальная ситуация, когда разработка прикладного ПО под все целевые платформы в 99 случаях из 100 делается под Windows в Visual Studio. Как это ни удивительно, но никто за рамками статистической погрешности не разрабатывает прикладные продукты для Линукса под Линуксом. Все разрабатывается под Windows и лишь интегрируется и тестируется на целевой платформе.


  1. aabzel Автор
    29.07.2022 00:32

    deleted


  1. TheCalligrapher
    29.07.2022 22:25
    +2

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

    Язык С сам по себе не поддерживает такого использования битовых полей. В спецификации битовых полей специально подчеркивается, что способ их размещения в памяти никак не оговаривается вообще. Подобное использование - implementation-dependent хак.

    Битовые поля были введены в язык для экономии памяти, а не для разбора фиксированных битовых карт.

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

    Очень спорный совет, опирающийся на формальную, а не не человеческую логику. В случаях, когда оба сравниваемых значения имеют одинаковый "приоритет" - да, это хорошая идея. Но в подавляющем большинстве случае роли сравниваемых значений ассиметричны: есть тот, "кого" мы сравниваем, и есть тот, "с кем" мы сравниваем. Это особенно выраженно когда первое значение постоянно меняется (напр., в процессе поиска), а последнее - более-менее фиксировано. Именно в таком порядке и стоит записывать сравнения: сначала "кого", потом "с кем". Такая запись намного лучше передает естественный ход человеческой мысли. Если в таком порядке записи придется использовать оператор >, то так тому и быть.