get rid of unneeded string types variety in far2l

[unxed]

Currently far2l's code is full of redundant string types and conversions between them. Actually it's the living answer to the question "why some people hate c or c++?" :) It would be great to remove such redundancy step by step to make code more newbie-friendly.

I'm starting with TCHARs removal in favor of WCHARs. We already have TCHAR defined as WCHAR in WinPort, and even if someone someday ports far2l back to Windows, he is very unlikely to want to make it ANSI again just for ancient Win95/98/Me fans. So we can do it boldly with no worry about any possible harm.

The first try is done on simple plugins so we can be sure nothing is broken during transition.

[elfmz]

sorry for oldschoolness, but before booth painting, lets first make a stable release that is at least beta

[unxed]

sorry for oldschoolness, but before booth painting, lets first make a stable release that is at least beta

What are the beta criteria, by the way?

[elfmz]

No critical functionality bugs causing potential data loss, like incorrect file operations (as #794) or lost terminal output, or heavy stucks/crashes without known adequate reasons, for a long enough time (couple of months).

[elfmz]

And of course, each serious enough change resets that 'couple of month' period:)

[unxed]

Do we still have such critical bugs besides #794 right now?

[elfmz]

yep, everything that 'failed to open archive/crashing on start etc. But may be some oldest of them can be ignored, if reason still unknown but nobody confirmed. Ando If reason is known and its hard to fix (kind of 7zip specific methods can't be extracted cuz auther of zi unpacker didnt inclyude code for them into unpacking library) - also can postpone to after-beta.

[elfmz]

Honestly, it probably could be marked as beta few weeks ago, but intuitively i didn't feel so sure that its was good enough for that, and now recently there were bunch potential risky changes, but its good that they came in same time, so if to break - better to break for a while, than constantly time from time.

[elfmz]

Regarding string types etc - its all simple problems, like all who worked with Windows API a bit - knows what is TCHAR. Also creating code documentation that is-detached from code itself - is quite a useless task - it will quickly became inadequate once something will change. Its better to write inline comments into code, so from one side - tools like doxygen would create code documentation in automated way, from other side - it will be much more up-to-date. But again - documenting whole bunch of old code is huge task, and however it should be done by someone who understand code well, cuz incorrect(misleading) documentation is worse that missing documentation.

[unxed]

like incorrect file operations (as #794)

please review #804 :)

[elfmz]

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

[unxed]

Ну, я пока не настолько крут, чтобы прям вот поддержку ZIP64 сходу довести до ума (какие-то её части там уже наблюдаются, но не очень понятно, насколько живые). А PR тут исключительно в качестве доказательства того, что я причину нашел правильно.

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

)))

[unxed]

К слову, список он правильно читает с этим патчиком, все файлы на месте. Этот хедер же идет в конце после всего — кажется, его игнорирование тут не катастрофично (ну разве что мы не можем ругануться, если он кривой). 65540 файлов даже показывает как миленький, и даже в пароленном архиве.

Но, может, я что-то и упускаю, конечно.

[elfmz]

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

[unxed]

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

https://github.com/CreeperKong/zipbomb-generator --zip64

:))

[unxed]

python3 zipbomb.py --zip64 --num-files=99999 --compressed-size=5000000 > zbsm.zip дает нам как раз то, что нужно для тестирования (да, увы, мы ломаемся на таком пока, а вот unzip, например, ест)

zbsm.zip тут 516 Тб в 99 999 файлах по 5 Гб (сам архивчик 16Mb)

[alk0]

Парни, моя задача тест-кейс оформить — баг и пруф, что мне не померещилось. И раз Zip64 не умеем, лучше посыпаться сразу, имхо, чем потом в продакшене файлы терять на ровном месте по непонятным причинам. Оно, конечно, немного подбешивает, что File And Archive Manager не умеет в архивы, но штош, бывает, не шмогла. (И архивы с >64k файлов вовсе не экзотика, если что.)

(Ещё б https://github.com/elfmz/far2l/issues/496 починили как-нибудь — вот уж что реально бесит до трясучки. Или может я слоупок, и есть какой-то секретный шорткат для этого? Низнаю.)

[unxed]

python3 zipbomb.py --zip64 --num-files=99999 --compressed-size=5000000 > zbsm.zip дает нам как раз то, что нужно для тестирования (да, увы, мы ломаемся на таком пока, а вот unzip, например, ест)

zbsm.zip тут 516 Тб в 99 999 файлах по 5 Гб (сам архивчик 16Mb)

Ошибка тут в ZIP_OpenArchive происходит, а именно, вот в этой строчке: WINPORT(ReadFile)(ArcHandle,&NextPosition.u.LowPart,sizeof(NextPosition.u.LowPart),&ReadSize,NULL);

Если там -1 приходит, надо это дело игнорировать и смотреть в ZIP64-заголовок 0x06064b50. Мы так пока не умеем.

Ну и если воткнуть туда костыль и таки открыть файл, ещё размеры файлов запакованных обрезатся по 32 битам для ZIP64 у нас, да. Кстати, смешно, unzip -l показывает размер каждого отдельного файла правильно, а вот сумму считает тоже по обрезанным по 32 битам размерам :)

[elfmz]

не надо костылей

[elfmz]

Ещё б #496 починили как-нибудь

Починил.. Ну как починил, отключил это из настроек по умолчанию. Самое забавное что настройка - есть, а ручки для ее изменения в UI вынести забыли.

[alk0]

Очень круто, спасибо, от души прям! А если не понравится, как обратно поменять? Ага, вроде разобрался (сырцы, дедукция, эксперимент!) — создаём ~/.config/far2l/REG/HKU/c/k-Software/k-Far2/k-System/v-QuotedName, внутри…

QuotedName
DWORD
1

… и это битовая маска (1 для командной строки, 2 для буфера). И вроде работает. ОК, теперь вопрос — а это уже документировано где-то? Я что-то не нашёл.

[unxed]

Процитирую отсюда:

elfmz: Велосипедные reference counted строки [речь про класс FARString — прим. unxed] имеют свое право на существование - память неплохо экономится, если открыть 100500 директорию файлов.

sizeof(std::wstring) - 32 байта, эксперименты показывают, что в gcc 11.4 в std::wstring строку можно положить максимум 3 символа до начала аллокаций. То есть оптимизация локального хранения строки для wstring фактически бесполезна - реальные строки всегда работают с реаллокациями при всяких копированиях, так как файлы <= 3 символов в имени редкость кроме пустых. Вот еслиб строки были в UTF8 этой проблемы бы не было, но имеем что имеем - 4хбайтный wchar_t.

С другой стороны пустые строки не редкость, и 32 байта вместо 8 байт делают разницу, для случая FileListItem к примеру, в котором есть часто пустой strCustomData. А еще там же есть strOwner, strGroup которые интернируются, то есть если 100500 файлов имеют один юзер (как оно и бывает обычно) то память под его имя выделяется один раз. Вобщем, както так, особо минусов изза двух разных строк кроме как 'неаккуратненько как-то' не видно, а плюсы все же есть.

unxed: А с одинаковым пользователем точно так работать будет? Там же получается экономия только при присовении одной FARString другой, а если нам от системного вызова какого-то пришёл десять раз один и тот же char*, то новые Content создадутся, не?

elfmz: А там есть такой классик, CachedFileLookupT который будучи юзаем в правильном месте (в FileList::ReadFileNames) копирует в себе каждый уникальый FARString и потом раздает его копии при совпадении. Именно чтоб сэкономить память.

@akruphi @ivanshatsky

[unxed]

sorry for oldschoolness, but before booth painting, lets first make a stable release that is at least beta

@elfmz у нас, кстати, таки beta :) Я только забыл, нам TCHAR'ы всё ещё нужны для чего-то?

[akruphi]

Вопрос про актуальность TCHAR в свежедобавляемом коде.

Сейчас доделываю редактирование параметров в far:config и столкнулся с тем, что DialogBuilder(FarLangMsg TitleMessageId, const wchar_t *HelpTopic); из far2l/src/DialogBuilder.hpp и AddText(FarLangMsg LabelId) из far2sdk/fardlgbuilderbase.hpp принимают только FarLangMsg, который фактически int.

Внутри затем через GetLangString() возвращаются const TCHAR *

Но для быстрого кодирования редактирования в far:config достаточно засылать в диалог на лету создаваемую строку.

Уважаемый @elfmz, если добавлять функции, принимающие строки, то в качестве аргументов тянуть использование TCHAR или сразу делать через const wchar_t *, т.е. если введу функции DialogBuilder(const wchar_t * TitleMessage, const wchar_t *HelpTopic); и AddText(const wchar_t * Label) это в нынешних условиях норм или историческая совместимость тут важнее и надо использовать тип const TCHAR *?

[elfmz]

принимают только FarLangMsg, который фактически int.

ну это совсем не int уже давно: https://github.com/elfmz/far2l/blob/master/far2l/src/lang.hpp#L39

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

[unxed]

так может вообще с TCHAR'ов на wchat_r переходить понемножку, как я предлагал изначально? ну реально неадекватный зоопарк строковых типов же, очень порог входа для новичков поднимает.

[elfmz]

кстати я в похожей ситуации такой костылек сделал: FarLangMsg{::Lang.InternMsg("arbitrary string")} но вообще в диалогах обычно строки предполагается переводить рано или поздно, и лучше уж сразу и медленно сделать нормально, чем быстро-быстро но с костылями и недопереводом интерфейса

[unxed]

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

[akruphi]

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

Понял. Уже немного локально закоммитил, но на git не заливал. Откачу и переделаю отдельными коммитами.

Пока в PR не заливаю, жду одобрите ли Вы #1840 или мне там переделывать.

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

В far3 как раз far:config сделан без перевода на другие языки и пока думаю и тут можно так оставить - ведь far:config даже не вызывается через меню, а только командой. Согласен с RUP подходом упомянутом @unxed - сделаем, чтоб работало, а потом обкатав варианты фичи (пока не уверен обезъянить ли для чисел быстро из far3 неудобный набор F4/Shift-F4/Alt-F4 или сделать как-нибудь более удобно) уже можно и мордочку переводную делать.

[elfmz]

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

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

[unxed]

Должен признать, что он и правда работает стабильно и невероятно удобен 🙏🙂

[unxed]

Хм, а может быть архитектурно правильным решением было бы сделать класс-надстройку над std::string с добавлением дедупликации? Ну в смысле от std::string наследоваться.

[elfmz]

sizeof(std::string) = 32, sizeof(FARString) = 8 sizeof(FARString отнаследованный от std::string) будет 40) . Да и толку с такого наследования - методы у std::string не виртуальные, потому оно не будет делать то что хочется. Вообще это не наследованием решается, а приведением апи к одинаковому виду, но у FARString есть полезняшки которых нету в std::string типа Format, CellsCount.. Мжно конечно их в виде отдельных функций сделать но тоже некрасиво будет.. Вощем не всегда рефакторинг ради рефакторинга хорошо.

[elfmz]

Причем замечу что std::string имеет размер 32 байта изза small string optimization, которая в случае фара с его wchar_t строками фактически бесполезна, тк только очень уж короткие строки - до 3х символов - могут влезить в локальный буфер строки, для строк остальных размеров 16 байт из этих 32х - просто бесполезный оверхед. Вобщем small string optimization применительно к std::wstring - это small string pessimization.