consistent formatting

[yanovich]

Huge whitespace-only change.

[beldmit]

Не-не-не... Автогенерированные файлы про эллиптику лучше не трогать

[yanovich]

Скажите какие -- уберу. Может добавить форматирования в генератор?

[beldmit]

ecp_id_*

[beldmit]

Результаты пока скорее странные. Чтоб я помнил, откуда взялся файл getopt.h Часть файлов, кажется, я форматировал openssl-ным скриптом когда-то.

[yanovich]

Часть файлов, кажется, я форматировал openssl-ным скриптом когда-то.

Только если очень небольшую часть )

$ list=$(git ls-tree --name-only -r HEAD | grep -v ecp_id_.* | grep '[.][ch]$'); ../util/check-format.pl $list
...
151049 (150032 indentation, 0 '#if' nesting indent, 3 syntax, 96 whitespace, 50 length, 868 other) issues have been found by ../util/check-format.pl

[vt-alt]

Странные замены навскидку:

- static const char *gost_envnames[] =
-     { "CRYPT_PARAMS", "GOST_PBE_HMAC", "GOST_PK_FORMAT" };
+ static const char *gost_envnames[] = {
+     "CRYPT_PARAMS", "GOST_PBE_HMAC", "GOST_PK_FORMAT"};

Уже писал в #426 - почему это отформатировалось так а следующее за ним эдак:

- const uint8_t grasshopper_pi_inv[0x100] = {
-         0xA5, 0x2D, 0x32, 0x8F, 0x0E, 0x30, 0x38, 0xC0,    // 00..07
-         0x54, 0xE6, 0x9E, 0x39, 0x55, 0x7E, 0x52, 0x91,    // 08..0F
...
+     0xA5, 0x2D, 0x32, 0x8F, 0x0E, 0x30, 0x38, 0xC0, // 00..07
+     0x54, 0xE6, 0x9E, 0x39, 0x55, 0x7E, 0x52, 0x91, // 08..0F
...
- const uint8_t grasshopper_lvec[16] = {
-         0x94, 0x20, 0x85, 0x10, 0xC2, 0xC0, 0x01, 0xFB,
-         0x01, 0xC0, 0xC2, 0x10, 0x85, 0x20, 0x94, 0x01
- };
+ const uint8_t grasshopper_lvec[16] = {0x94,
+                                       0x20,
+                                       0x85,

Иногда аргументы выровнены по краю открывающей скобки, а иногда нет:

-     return EVP_Cipher(ctx->actx, ctx->km, zero_iv,
-         EVP_CIPHER_key_length(EVP_CIPHER_CTX_cipher(ctx->actx)) +
-         EVP_CIPHER_CTX_block_size(ctx->cctx));
+     return EVP_Cipher(ctx->actx,
+                       ctx->km,
+                       zero_iv,
+                       EVP_CIPHER_key_length(EVP_CIPHER_CTX_cipher(ctx->actx))
+                           + EVP_CIPHER_CTX_block_size(ctx->cctx));
+ }

-     gost2012_hash_block((gost2012_hash_ctx *) EVP_MD_CTX_md_data(ctx), data,
-                         count);
+     gost2012_hash_block(
+         (gost2012_hash_ctx *)EVP_MD_CTX_md_data(ctx), data, count);

-         while (get_line
-                (check_file, inhash, filename, verbose, &expected_hash_size)) {
+         while (get_line(
+             check_file, inhash, filename, verbose, &expected_hash_size)) {

Вернуло одну проверку в предыдущею строку (видимо потому что она влезала в 80 символов) хотя все остальные в столбец:

      if ((ctx = OPENSSL_zalloc(sizeof(*ctx))) != NULL
          && (ctx->proverr_handle = proverr_new_handle(core, in)) != NULL
          && (ctx->libctx = OSSL_LIB_CTX_new()) != NULL
-         && (ctx->e = ENGINE_new()) != NULL
-         && populate_gost_engine(ctx->e)) {
+         && (ctx->e = ENGINE_new()) != NULL && populate_gost_engine(ctx->e)) {
          ctx->core_handle = core;

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

[yanovich]

Уже писал в #426 - почему это отформатировалось так а следующее за ним эдак:

Тут не знаю, но думаю, что из-за комментариев.

Иногда аргументы выровнены по краю открывающей скобки, а иногда нет:

Это AllowAllArgumentsOnNextLine. Можно отключить.

Вернуло одну проверку в предыдущею строку (видимо потому что она влезала в 80 символов) хотя все остальные в столбец:

Это странное поведение.

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

Для аргументов AllowAllArgumentsOnNextLine, по умолчанию true. и BinPackArguments: false. Поэтому или одна строка, или по одному.

[yanovich]

До патча:

list=$(git ls-tree --name-only -r HEAD | grep -v ecp_id_.* | grep -v gost_grasshopper_precompiled.c | grep '[.][ch]$'); ../util/check-format.pl $list
...
8898 (4019 indentation, 0 '#if' nesting indent, 3 syntax, 541 whitespace, 390 length, 3945 other) issues have been found by ../util/check-format.pl

С патчем:

list=$(git ls-tree --name-only -r HEAD | grep -v ecp_id_.* | grep -v gost_grasshopper_precompiled.c | grep '[.][ch]$'); ../util/check-format.pl $list
...
3272 (2383 indentation, 0 '#if' nesting indent, 3 syntax, 94 whitespace, 49 length, 743 other) issues have been found by ../util/check-format.pl

Решение не идеальное, но в правильном направлении. Лучше ничего нет, всё равно. Только если руками править все 8898 ошибкок формативания.

[beldmit]

gost_err* генерируются OpenSSL-ным скриптом. Собственно, их лучше перегенерировать, и если их формат не совпадают с форматированием по скрипту, то его оставить.

Я с подозрением смотрю на getopt.h, но пока проще переформатировать, чем убивать.

В остальном меня всё скорее устраивает. @vt-alt?

[beldmit]

Ад какой-то :( Переформатирование по аргументу на строчку совсем лишнее.

Можем мы это как-то приблизить к openssl-ному формату, или идти постепенно, для начала - с пробелами и табуляциями?

[yanovich]

Ад какой-то :( Переформатирование по аргументу на строчку совсем лишнее.

Можно поменять для вызовов функций.

Можем мы это как-то приблизить к openssl-ному формату, или идти постепенно, для начала - с пробелами и табуляциями?

У OpenSSL же нет своего формативания. Только проверка. В linux и node.js тоже используют clang-format. Промышленных альтернтив нет, насколько я понимаю. Было что-то на основе gcc, но не взлетело. Так что варианта 2: или всё руками, или пользоваться тем, что есть.

[yanovich]

Попробовал BinPackArguments:true. Массивы стали выглядеть лучше, но ошибок при проверке скриптом от opensslстало больше. Видимо, для некоторых массивов, инициализация востпринимается clang'ом как вызов конструктора. Я только так могу предположить, почему этот параметр влияет на форматирование инициализации массивов.

3362 (2467 indentation, 0 '#if' nesting indent, 3 syntax, 94 whitespace, 53 length, 745 other) issues have been found by ../util/check-format.pl

[vt-alt]

Если какой-то скрипт будет заставлять человека менять форматирование на такое какое генерирует clang-fiormat, то как человек должен заранее понять, что в одном месте надо было писать аргументы пока влезут до 80го столбца, в другом вертикально по одному, а в третьем 6й и 7й из них склеить в одну строку, в одном месте надо сделать отступ от скобки, а другом от начала предыдущей строки?

И это без учета, что теряется git blame — за такую потерю в анализе кода, который писали много лет, надо хоть что-то приобрести. А затраты труда на поломать то что есть — минуты?

У OpenSSL же нет своего формативания. Только проверка. В linux и node.js тоже используют clang-format.

В Linux clang-format не енфорсится,, а предлагается как утилита в помощь в форматировании. https://www.kernel.org/doc/html/latest/process/clang-format.html

Так что варианта 2: или всё руками, или пользоваться тем, что есть.

Заменять кривое ручное форматирование, на кривое машинное как-то странно. Если мы теряем git blame, то хотя бы оно должно выглядеть хорошо.

[vt-alt]

Только сейчас прочитал про .git-blame-ignore-revs , интересно, на сколько хорошо это работает.

[yanovich]

Только сейчас прочитал про .git-blame-ignore-revs , интересно, на сколько хорошо это работает.

новые пустые строчки остаются только. https://github.com/gost-engine/engine/blame/5e777b8b3de7f0eee09054f1ecd9cd0993ee4762/gost_eng.c

[yanovich]

Если какой-то скрипт будет заставлять человека менять форматирование на такое какое генерирует clang-fiormat, то как человек должен заранее понять, что в одном месте надо было писать аргументы пока влезут до 80го столбца, в другом вертикально по одному, а в третьем 6й и 7й из них склеить в одну строку, в одном месте надо сделать отступ от скобки, а другом от начала предыдущей строки?

Например, можно просто сделать $ ./.github/format.sh. Но обычно люди ставят интеграцию в свой редактор (см. соответствующий раздел), через некоторое время вызов форматирования становится рефлекторным, и проекты, где им нельзя пользоваться, начинают очущаться какими-то опасными.

У OpenSSL же нет своего формативания. Только проверка. В linux и node.js тоже используют clang-format.

В Linux clang-format не енфорсится,, а предлагается как утилита в помощь в форматировании. https://www.kernel.org/doc/html/latest/process/clang-format.html

Там форсится check-patch.pl. Имеется опыт ручного формативания файла с новым драйвером в до-клангформатное время ).

И линукс, и опенссл проходят аудит кода в разных нерусских трёхбуквенных организациях, и им надо там каждую строчку каждого патча объяснять, поэтому там не будет такого PR, как этот. Но это же не достоинство, а как раз наоборот.

[vt-alt]

Там форсится check-patch.pl. Имеется опыт ручного формативания файла с новым драйвером в до-клангформатное время ).

~/linux/torvalds (master)$ grep -ic clang scripts/checkpatch.pl
0

Да и перед постом я грепнул, что clang-format упоминается только в документации (просмотрел её ещё раз) и в коде в строках типа /* clang-format off */.

[yanovich]

У OpenSSL же нет своего формативания. Только проверка. В linux и node.js тоже используют clang-format.

В Linux clang-format не енфорсится,, а предлагается как утилита в помощь в форматировании. https://www.kernel.org/doc/html/latest/process/clang-format.html

Я нигде не утверждал, что clang-format в linux "енфорсится". Факт "использования" его в линуксе отрицать бессмысленно. Основная мысль моего комментария, который Вы процитировали: "скриптов на стиль в мире как грязи, а промышленых алтернатив клангформату нет". А нет их потому, что для разработки такой утилиты, чтобы можно было с закрытыми глазами ей пользоваться, надо писать компилятор С, или использовать уже готовый.