[#] Красота против простоты
hugeping(ping,1) — All
2024-12-27 13:49:06


Разбирал известные CVE для ядра Linux и в процессе натолкнулся на такое изменение:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=0f41f383b5a61a2bf6429a449ebba7fb08179d81

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 3a4cefb25ba61c..9ca4211c063f39 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -1124,10 +1124,9 @@ static inline int parse_perf_domain(int cpu, const char *list_name,
 				    const char *cell_name,
 				    struct of_phandle_args *args)
 {
-	struct device_node *cpu_np;
 	int ret;

-	cpu_np = of_cpu_device_node_get(cpu);
+	struct device_node *cpu_np __free(device_node) = of_cpu_device_node_get(cpu);
 	if (!cpu_np)
 		return -ENODEV;

@@ -1135,9 +1134,6 @@ static inline int parse_perf_domain(int cpu, const char *list_name,
 					 args);
 	if (ret < 0)
 		return ret;
-
-	of_node_put(cpu_np);
-
 	return 0;
 }

В этом изменении мы видим что-то напоминающее конструкцию defer в golang. А именно:

> struct device_node *cpu_np __free(device_node) = of_cpu_device_node_get(cpu);

Я до сих пор не сталкивался с подобными штуками в Си, поэтому сразу же начал изучать:

https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-cleanup-variable-attribute

Оказалось, что в gcc/clang есть такой атрибут __cleanup__, с помощью которого можно написать что-то вроде:

void *ptr __attribute__((__cleanup__(free))) = NULL;
...
ptr = malloc(32);

И после того как ptr уйдёт из зоны видимости, вызовется free() для ptr. "А что, так можно было? Здорово!" -- подумал я. Все эти цепочные goto при обработке ошибок в ядре действительно выглядят не очень. И начал смотреть как это используется в ядре... Вернёмся к строке:

> struct device_node *cpu_np __free(device_node) = of_cpu_device_node_get(cpu);

__free -- вероятно, макрос устанавливающий атрибут. Но какая функция вызовется для освобождения? Из кода этого не видно... ведь device_node это структура. Смотрим макрос __free:

> #define __free(_name) __cleanup(__free_##_name)

Ок. Смотрим макрос __cleanup:

> #define __cleanup(func) __attribute__((__cleanup__(func)))

Ага, вот наш атрибут. В итоге мы понимаем, что вызовется функция __free_device_node. Не очень понятно зачем генерация имени спрятана внутри макроса? Ищем __free_device_node и ... не находим! Макросы... Сразу над __cleanup макросом определён другой:

#define DEFINE_FREE(_name, _type, _free) \
	static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }

Ох, теперь понятно. Где-то в коде есть DEFINE_FREE(device_node, ...

Находим (в другом .h файле):

> DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T))

Ура! Мы прошли весь путь! Честно говоря, меня все эти макросы утомили. Неужели они упрощают код? Мне кажется, наоборот. Да, вроде бы всё становится красиво, но глядя на код и не зная во что разворачивается тот или иной макрос мы вообще ничего не понимаем. Когда мы используем defer в Go мы пишем явно. Неужели запись вида:

> struct device_node *cpu_np __free(cleanup_device_node) = of_cpu_device_node_get(cpu);

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

На самом деле, есть статья где прямым текстом описан механизм cleanup в ядре:

Scope-based resource management for the kernel: https://lwn.net/Articles/934679/

Так что можно было бы не копаться в коде, а прочитать статью. Но это не наш путь. :)

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

Когда я ковырялся с кодом Plan9 я был в шоке встречающихся констант прямо в коде. Что-нибудь вроде:

> a = bufimage(dst->display, 1+4+4+4+4*4+2*4+2*4);

Страшно? :) Мне тоже! Я привык, что в таком случае мы должны явно написать или несколько #define, или определить структуру или явно вписывать sizeof(). Я и сейчас так думаю. Но... глядя на эту конструкцию я не могу не отметить её полную прозрачность для читающего. И незаметно для себя стал реже делать определение констант, по крайней мере в тех случаях, когда константа используется всего один раз. Либо определять константу непосредственно в месте, где она используется.

И выбирая между sizeof(uint32_t) и 4 я всё чаще (к своему ужасу) выбираю 4.