Не к работе сейчас. Разбор и план на будущее: браться после того, как закончится перевод на UTF-8 (#3787, #3788). Решение о том, делать ли это вообще и в каком порядке, — за Стрибогом.
После перехода на UTF-8 (#3681) выяснилось, что почти каждая ошибка вывода за последние недели растёт из одного корня: текст в движке живёт в сырых char-буферах фиксированного размера, и код обращается с ними как с массивом байт. Пока байт равнялся символу, это работало. Теперь — нет.
Что уже пришлось чинить по этой причине: переполнение буфера в «где» (#3681), падение на осмотре ёмкости (#3752), ширина колонок в 40+ местах (#3797), «статус» со своим набором букв (#3795), ответ «нет» в zedit (#3796), %c вместо буквы (#3797), заглавная буква в act() (#3806). Каждый раз — точечная правка одного места.
Предлагаю перестать латать и наметить план.
Что имеем
|
сколько |
char * в объявлениях |
3508 в 646 файлах |
sprintf |
1416 в 178 файлах |
snprintf |
1244 в 125 файлах |
локальные char buf[N] |
695 в 171 файле |
strcat |
387 в 37 файлах |
strcpy |
237 в 66 файлах |
str_dup |
100 в 36 файлах |
Отдельно — пять глобальных буферов (utils.h:638):
extern char buf[kMaxStringLength];
extern char buf1[kMaxStringLength];
extern char buf2[kMaxStringLength];
extern char arg[kMaxInputLength];
extern char smallBuf[kMaxRawInputLength];
Их используют 197 файлов, 4061 раз. Это не просто «старый стиль»: буфер общий на весь процесс, поэтому вызов чужой функции посреди сборки строки молча затирает то, что уже накопили. Плюс plant_magic/test_magic — сторожевой байт в конце, которым ловят переполнение уже постфактум.
Ещё 107 объявлений — буферы до 128 байт. В UTF-8 это около 42 русских букв; ровно на таком в #3752 и падало.
Чем плохо именно сейчас
- Байт ≠ символ. Любая арифметика по длине (
strlen как «сколько букв», обрезка по индексу, %-20.20s) теперь врёт.
- Переполнение вместо ошибки. UTF-8 удлиняет русский текст вдвое; буфер, которого хватало, внезапно мал — и это падение процесса, а не сообщение в лог.
- Глобальные буферы связывают код. Нельзя вынести функцию, нельзя вызвать её из другого места, нельзя распараллелить — всё завязано на общий
buf.
План
Не переписывать всё разом: 3500 мест — это месяцы и гарантированные регрессии. Двигаться слоями, каждый слой самостоятелен и проверяем.
Слой 0. Договориться о правилах (одна страница в CONTRIBUTING)
- Новый код не использует глобальные
buf*, sprintf, strcat, strcpy.
- Строки собираются
fmt::format, передаются const std::string & или std::string_view.
- Где нужна ширина/обрезка — только
fmt или native_text::*, никогда %-Ns и не по индексу.
Без этого поток нового кода будет ровно такой же, и уборка не сойдётся.
Слой 1. Убрать глобальные буферы из «листьев» (197 файлов, по одному)
Начинать с файлов, где буфер используется локально и не переживает вызов: команды игрока и богов. Порядок по объёму — от простых:
| файл |
вхождений |
do_who.cpp |
79 |
do_set.cpp / do_set_all.cpp |
61 / 62 |
corpse.cpp |
52 |
named_stuff.cpp |
85 |
punishments.cpp |
98 |
identify.cpp |
149 |
do_score.cpp |
137 |
do_show.cpp |
97 |
do_stat.cpp |
350 |
dg_scripts.cpp |
479 |
Правка механическая: char buf[] → локальная std::string, sprintf → fmt::format, strcat → +=. Образцы уже есть — 11 файлов полностью без sprintf/strcat/strcpy, например vedun.cpp (60 вызовов fmt::format), mapsystem.cpp, boards.cpp, do_inspect.cpp.
Признак готовности слоя: extern char buf[] можно удалить из utils.h.
Слой 2. Функции, которые возвращают char *
Их около сотни (str_dup и статические буферы вроде diag_obj_to_char). Каждая — источник вопросов «кто освобождает» и «когда затрётся». Переводить на std::string по одной, вместе с вызывающими.
Слой 3. Границы, где char * останется
act(), интерпретатор команд, парсеры мира, сеть — там char * не случайность, а формат данных. Здесь цель скромнее: не переписывать, а обложить проверками — вход через std::string_view, длины через native_text::*, никакой байтовой арифметики над текстом.
Слой 4. Маленькие буферы
107 объявлений до 128 байт пересмотреть отдельно: часть заменить на std::string, часть просто увеличить втрое, если это горячий путь и аллокация нежелательна.
Как мерить прогресс
grep -rc 'sprintf\|strcat\|strcpy' src --include=*.cpp | awk -F: '{s+=$2} END {print s}'
и число файлов, где ещё видны глобальные buf*. Обе цифры должны монотонно убывать; вводить новые — только через исключение в ревью.
Чего НЕ делать
- Не переводить всё автоматом:
sprintf в char[], который потом уходит в act() или в парсер, менять нельзя без разбора вызывающих.
- Не гнаться за скоростью:
fmt::format быстрее printf без ширины (~104 нс против 123 нс) и медленнее с ней (250 против 87). Это не аргумент ни за, ни против — обе величины ничтожны на фоне пульса.
- Не трогать
%c, %d, %s без ширины над ASCII — там printf корректен, правка будет шумом.
Порядок
Предлагаю начать со слоя 0 (правила) и первых трёх-четырёх файлов слоя 1, чтобы отработать приём и посмотреть на размер диффа. Дальше — по файлу за заход, каждый отдельным PR: так регрессии видны сразу и откат дешёвый.
После перехода на UTF-8 (#3681) выяснилось, что почти каждая ошибка вывода за последние недели растёт из одного корня: текст в движке живёт в сырых
char-буферах фиксированного размера, и код обращается с ними как с массивом байт. Пока байт равнялся символу, это работало. Теперь — нет.Что уже пришлось чинить по этой причине: переполнение буфера в «где» (#3681), падение на осмотре ёмкости (#3752), ширина колонок в 40+ местах (#3797), «статус» со своим набором букв (#3795), ответ «нет» в zedit (#3796),
%cвместо буквы (#3797), заглавная буква вact()(#3806). Каждый раз — точечная правка одного места.Предлагаю перестать латать и наметить план.
Что имеем
char *в объявленияхsprintfsnprintfchar buf[N]strcatstrcpystr_dupОтдельно — пять глобальных буферов (
utils.h:638):Их используют 197 файлов, 4061 раз. Это не просто «старый стиль»: буфер общий на весь процесс, поэтому вызов чужой функции посреди сборки строки молча затирает то, что уже накопили. Плюс
plant_magic/test_magic— сторожевой байт в конце, которым ловят переполнение уже постфактум.Ещё 107 объявлений — буферы до 128 байт. В UTF-8 это около 42 русских букв; ровно на таком в #3752 и падало.
Чем плохо именно сейчас
strlenкак «сколько букв», обрезка по индексу,%-20.20s) теперь врёт.buf.План
Не переписывать всё разом: 3500 мест — это месяцы и гарантированные регрессии. Двигаться слоями, каждый слой самостоятелен и проверяем.
Слой 0. Договориться о правилах (одна страница в CONTRIBUTING)
buf*,sprintf,strcat,strcpy.fmt::format, передаютсяconst std::string &илиstd::string_view.fmtилиnative_text::*, никогда%-Nsи не по индексу.Без этого поток нового кода будет ровно такой же, и уборка не сойдётся.
Слой 1. Убрать глобальные буферы из «листьев» (197 файлов, по одному)
Начинать с файлов, где буфер используется локально и не переживает вызов: команды игрока и богов. Порядок по объёму — от простых:
do_who.cppdo_set.cpp/do_set_all.cppcorpse.cppnamed_stuff.cpppunishments.cppidentify.cppdo_score.cppdo_show.cppdo_stat.cppdg_scripts.cppПравка механическая:
char buf[]→ локальнаяstd::string,sprintf→fmt::format,strcat→+=. Образцы уже есть — 11 файлов полностью безsprintf/strcat/strcpy, напримерvedun.cpp(60 вызововfmt::format),mapsystem.cpp,boards.cpp,do_inspect.cpp.Признак готовности слоя:
extern char buf[]можно удалить изutils.h.Слой 2. Функции, которые возвращают
char *Их около сотни (
str_dupи статические буферы вродеdiag_obj_to_char). Каждая — источник вопросов «кто освобождает» и «когда затрётся». Переводить наstd::stringпо одной, вместе с вызывающими.Слой 3. Границы, где
char *останетсяact(), интерпретатор команд, парсеры мира, сеть — тамchar *не случайность, а формат данных. Здесь цель скромнее: не переписывать, а обложить проверками — вход черезstd::string_view, длины черезnative_text::*, никакой байтовой арифметики над текстом.Слой 4. Маленькие буферы
107 объявлений до 128 байт пересмотреть отдельно: часть заменить на
std::string, часть просто увеличить втрое, если это горячий путь и аллокация нежелательна.Как мерить прогресс
и число файлов, где ещё видны глобальные
buf*. Обе цифры должны монотонно убывать; вводить новые — только через исключение в ревью.Чего НЕ делать
sprintfвchar[], который потом уходит вact()или в парсер, менять нельзя без разбора вызывающих.fmt::formatбыстрееprintfбез ширины (~104 нс против 123 нс) и медленнее с ней (250 против 87). Это не аргумент ни за, ни против — обе величины ничтожны на фоне пульса.%c,%d,%sбез ширины над ASCII — тамprintfкорректен, правка будет шумом.Порядок
Предлагаю начать со слоя 0 (правила) и первых трёх-четырёх файлов слоя 1, чтобы отработать приём и посмотреть на размер диффа. Дальше — по файлу за заход, каждый отдельным PR: так регрессии видны сразу и откат дешёвый.