Bug 35797 - Сломано отображение раскладок
Summary: Сломано отображение раскладок
Status: CLOSED FIXED
Alias: None
Product: Sisyphus
Classification: Development
Component: alterator-xkb (show other bugs)
Version: unstable
Hardware: all Linux
: P3 critical
Assignee: manowar@altlinux.org
QA Contact: qa-sisyphus
URL:
Keywords:
Depends on:
Blocks: 33000
  Show dependency tree
 
Reported: 2018-12-19 17:52 MSK by Антон Мидюков
Modified: 2019-05-28 17:36 MSK (History)
7 users (show)

See Also:


Attachments
сломанное отображаение раскладок (110.54 KB, image/png)
2018-12-19 17:52 MSK, Антон Мидюков
no flags Details
Отладочный вывод перед (widget rows labels) (1.85 KB, text/plain)
2019-01-31 01:22 MSK, manowar@altlinux.org
no flags Details
Команда для заполнения выпадающего списка значений (1.67 KB, text/xml)
2019-01-31 02:08 MSK, manowar@altlinux.org
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Антон Мидюков 2018-12-19 17:52:40 MSK
Created attachment 7910 [details]
сломанное отображаение раскладок

Сломано отображение раскладок (смотреть скрин во вложении). Сломалось в период 27.09.2018-23.10.2018. Сам alterator-xkb не обновлялся в этот период.
Comment 1 Ivan A. Melnikov 2019-01-30 16:06:42 MSK
Забавный баг, висящий на nobody.

Проблема с теми grp, у которых в описании есть ';'. Как-то похоже на фронтенде эта ';' обрабатывается особым образом. Проверил, добавление строчки 

sed -i 's/;/,/g' options

в конец функции dump_cache снимает проблему, но это какой-то совсем костыльный костыль.

Есть подозрение, что это связано с изменением в самом альтераторе:

* Wed Aug 29 2018 Paul Wolneykien <manowar at altlinux.org> 5.3-alt1
- Allow semicolon-separated values for the *-list types.

manowar@, не посмотрите, что тут лучше сделать?

Отмечу, что описания grp (вариантов переключения раскладки) прилетают из пакета xkeyboard-config. Там ';' в них появились в коммите 10cf5acd90327a0b9e115964c8c4f52b4575cd4b, который вошел в 2.25-alt1, попавший в Сизиф 15 октября 2018, что соответсвует указанному периоду возникновения проблемы.
Comment 2 manowar@altlinux.org 2019-01-30 16:11:43 MSK
> Есть подозрение, что это связано с изменением в самом альтераторе:
>
> * Wed Aug 29 2018 Paul Wolneykien <manowar at altlinux.org> 5.3-alt1
> - Allow semicolon-separated values for the *-list types.

Очень похоже.
Comment 3 manowar@altlinux.org 2019-01-31 01:22:29 MSK
Created attachment 7976 [details]
Отладочный вывод перед (widget rows labels)
Comment 4 manowar@altlinux.org 2019-01-31 01:23:02 MSK
Уже меньше похоже. Я вставил отладочный вывод вот сюда:

(cond
  ((member t '("listbox" "combobox"))
    (let* ((r (or (widget row) '#((label . ""))))
           (labels (map (lambda(x) (enumref-fill-row x r))
                        variants)))
---------------->>>>
      (widget rows labels)))

и получил вот такой, вполне пристойный список названий (см. файл xkb.labels).
Comment 5 manowar@altlinux.org 2019-01-31 02:08:19 MSK
Created attachment 7977 [details]
Команда для заполнения выпадающего списка значений
Comment 6 manowar@altlinux.org 2019-01-31 02:25:23 MSK
Насколько я могу судить вот по этой XMLке (https://bugzilla.altlinux.org/attachment.cgi?id=7977), элементы списка можно распарсить, потому что они отделены друг от друга ;; — двойным знаком точки с запятой, а не одинарным (кроме последнего). Однако мне, признаться, неизвестно, был ли такой формат передачи значений для браузера всегда или что-то действительно сломалось. Сергей?
Comment 7 Sergey V Turchin 2019-01-31 11:28:30 MSK
(В ответ на комментарий №6)
> отделены друг от друга ;; — двойным знаком точки с
> запятой, а не одинарным (кроме последнего).
Не двойной, а два одинарных. Если пустые элементы игнорировать, поплывёт.
Comment 8 manowar@altlinux.org 2019-01-31 11:34:23 MSK
Не понял. Что исправлять-то?
Comment 9 Sergey V Turchin 2019-01-31 11:52:37 MSK
Видимо, уже поплыло.
Попробуй сравнить с xml-ом на p8.
Comment 10 Sergey V Turchin 2019-01-31 12:31:47 MSK
(В ответ на комментарий №5)
> Команда для заполнения выпадающего списка значений
Не хватает ";" в некоторых местах.
После "Caps Lock на первую раскладку" и "Левая Win на первую раскладку", например.
Comment 11 Ivan A. Melnikov 2019-01-31 12:37:30 MSK
(In reply to comment #10)
> (В ответ на комментарий №5)
> > Команда для заполнения выпадающего списка значений
> Не хватает ";" в некоторых местах.
> После "Caps Lock на первую раскладку" и "Левая Win на первую раскладку",
> например.

Это не нехватает. Это пункт списка, содержащий ';', которую надо отобразить в этом пункте как ';'.
Comment 12 Sergey V Turchin 2019-01-31 14:58:32 MSK
(В ответ на комментарий №11)
> надо отобразить в этом пункте как ';'.
Зачем её там отображать?
Comment 13 manowar@altlinux.org 2019-01-31 15:02:19 MSK
Не о том спор, по-моему. Что сломалось-то и где? Это регресс или нет?
Comment 14 Sergey V Turchin 2019-01-31 15:06:42 MSK
(В ответ на комментарий №13)
> Не о том спор, по-моему. Что сломалось-то и где? Это регресс или нет?
На сколько я теперь понял, лишние знаки ";". Из-за них колбасит список, т.к. это разделитель.
Comment 15 Ivan A. Melnikov 2019-01-31 15:17:12 MSK
(In reply to comment #12)
> (В ответ на комментарий №11)
> > надо отобразить в этом пункте как ';'.
> Зачем её там отображать?

В данном случае это не разделитель item'ов в select, а часть описания способа переключения раскладки.

In english: "Caps Lock to first layout; Shift+Caps Lock to last layout"

http://git.altlinux.org/gears/x/xkeyboard-config.git?p=xkeyboard-config.git;a=blob;f=rules/base.xml.in;h=de88d403b23e29ae6f3d74d8459960f184d60463;hb=9793f3bed0e3ef4fefe3e2d6f46e9d222bc68caf#l6195

In russian:  "Caps Lock на первую раскладку; Shift+Caps Lock на последнюю раскладку"

http://git.altlinux.org/gears/x/xkeyboard-config.git?p=xkeyboard-config.git;a=blob;f=po/ru.po;h=27a1251f04f0fb82378e156b99603cf1923e1072;hb=9793f3bed0e3ef4fefe3e2d6f46e9d222bc68caf#l3530

Как я писал выше, можно подхачить бекенд и заменить ';' на ','. sed-ом. Если альтератор не способен отобразить пукты select, содержащие ';', надо так и сделать. Если способен, надо поправить где-то ещё чтобы такой случай корректно обрабатывался.
Comment 16 Sergey V Turchin 2019-01-31 15:21:09 MSK
(В ответ на комментарий №15)
> Как я писал выше, можно подхачить бекенд и заменить ';' на ','. sed-ом. Если
> альтератор не способен отобразить пукты select, содержащие ';', надо так и
> сделать.
';' считается разделителем и всё тут. Т.е. только на другой символ заменить.
Comment 17 Sergey V Turchin 2019-01-31 15:25:19 MSK
Еще там встречяаются всякие '&lt;'. Посмотрите, может заэскейпить получится.
Comment 18 manowar@altlinux.org 2019-01-31 16:19:42 MSK
(В ответ на комментарий №16)
> (В ответ на комментарий №15)
> > Как я писал выше, можно подхачить бекенд и заменить ';' на ','. sed-ом. Если
> > альтератор не способен отобразить пукты select, содержащие ';', надо так и
> > сделать.
> ';' считается разделителем и всё тут. Т.е. только на другой символ заменить.

Сергей, что именно является разделителем — одна (;) или только две (;;) ? Судя по XML, который я прикрепил выше, alterator-lookout полагает, что разделителем элементов списка является ;; — две тзпт. alterator-browser-qt считает иначе? Если да, то почему и когда это началось? Просто Антон завёл багу так, как будт-то бы это регресс и раньше всё работало.

Антон, на какой версии работает? p8?
Comment 19 Sergey V Turchin 2019-01-31 16:25:35 MSK
(В ответ на комментарий №18)
> Сергей, что именно является разделителем — одна (;) или только две (;;) ?
Одна.

> Судя
> по XML, который я прикрепил выше, alterator-lookout полагает, что разделителем
> элементов списка является ;; — две тзпт.
Нет, не полагает. Одна.
Там присутствует обязательный элемент. В данном случае он пустой.
Comment 20 manowar@altlinux.org 2019-01-31 16:27:04 MSK
> (В ответ на комментарий №18)
> Там присутствует обязательный элемент. В данном случае он пустой.

А-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а-а… Теперь дошло!
Comment 21 Антон Мидюков 2019-01-31 16:34:26 MSK
(В ответ на комментарий №18)
> Сергей, что именно является разделителем — одна (;) или только две (;;) ? Судя
> по XML, который я прикрепил выше, alterator-lookout полагает, что разделителем
> элементов списка является ;; — две тзпт. alterator-browser-qt считает иначе?
> Если да, то почему и когда это началось? Просто Антон завёл багу так, как
> будт-то бы это регресс и раньше всё работало.
> 
> Антон, на какой версии работает? p8?

alterator-browser-qt5-3.0.5-alt1.M80P.1, это та же версия, что и в Сизифе (%ubt).

iv@ же вполне точно определил, когда это началось:

>Отмечу, что описания grp (вариантов переключения раскладки) прилетают из пакета
>xkeyboard-config. Там ';' в них появились в коммите
>10cf5acd90327a0b9e115964c8c4f52b4575cd4b, который вошел в 2.25-alt1, попавший в
>Сизиф 15 октября 2018, что соответсвует указанному периоду возникновения
>проблемы.

15 октября 2018 началось. xkeyboard-config виноват, даёт нам то, что некорректно понимает alterator-browser-qt5
27.09.2018 точно работало. Сохранилась регулярка.
Comment 22 Sergey V Turchin 2019-01-31 16:45:09 MSK
(В ответ на комментарий №21)
> xkeyboard-config виноват, даёт нам то, что некорректно понимает
> alterator-browser-qt5
Не. Скорее, apterator-xkb некорректно передаёт.
alterator-browser-qt5 понимает ровно то, что ему дали.
Comment 23 Антон Мидюков 2019-01-31 16:50:46 MSK
(В ответ на комментарий №22)
> (В ответ на комментарий №21)
> > xkeyboard-config виноват, даёт нам то, что некорректно понимает
> > alterator-browser-qt5
> Не. Скорее, apterator-xkb некорректно передаёт.
> alterator-browser-qt5 понимает ровно то, что ему дали.

Согласен.

alterator-xkb фактически не менялся с 2009 года. Устарел...
Comment 24 Sergey V Turchin 2019-01-31 16:53:34 MSK
Да они оба как бы не менялись в этом плане, но входные данные обрабатывает alterator-xkb, а формат передачи к alterator-browser-qt изменению не подлежит.
Comment 25 manowar@altlinux.org 2019-01-31 17:39:38 MSK
(В ответ на комментарий №24)
> Да они оба как бы не менялись в этом плане, но входные данные обрабатывает
> alterator-xkb, а формат передачи к alterator-browser-qt изменению не подлежит.

Ок, формат разделения на элементы менять не будем. Придумай, однако, на что заменить ";" в передаче, чтобы пользователь на экране увидел именно его. Я поставлю соответствующий фильтр в alterator-lookout.
Comment 26 Sergey V Turchin 2019-01-31 17:55:37 MSK
(В ответ на комментарий №17)
> Еще там встречяаются всякие '&lt;'. Посмотрите, может заэскейпить получится.
Comment 27 manowar@altlinux.org 2019-01-31 18:59:11 MSK
(В ответ на комментарий №26)
> (В ответ на комментарий №17)
> > Еще там встречяаются всякие '&lt;'. Посмотрите, может заэскейпить получится.

&lt; — это символ "меньше" (<). А я про точку с запятой. Можно код символа указать через &#59; Пойдёт так?
Comment 28 manowar@altlinux.org 2019-02-01 01:27:48 MSK
(В ответ на комментарий №27)
> &lt; — это символ "меньше" (<). А я про точку с запятой. Можно код символа
> указать через &#59; Пойдёт так?

Хотя нет, не пойдёт. Такая замена сама содержит в себе ";". Нужно что-то другое, чтобы заменялось на ";" внутри самого alterator-browser-qt, после разбора XML.
Выбирай.
Comment 29 Sergey V Turchin 2019-02-01 12:25:06 MSK
(В ответ на комментарий №28)
> Хотя нет, не пойдёт. Такая замена сама содержит в себе ";". Нужно что-то
> чтобы заменялось на ";" внутри самого alterator-browser-qt, после разбора XML.
Т.к. alterator-browser-qt ничего не знает о передаваемых данных, то трогать ничего не будет. Только передавать туда-обратно, как до сих пор делал.
Comment 30 manowar@altlinux.org 2019-02-01 13:10:35 MSK
(В ответ на комментарий №29)
> (В ответ на комментарий №28)
> > Хотя нет, не пойдёт. Такая замена сама содержит в себе ";". Нужно что-то
> > чтобы заменялось на ";" внутри самого alterator-browser-qt, после разбора XML.
> Т.к. alterator-browser-qt ничего не знает о передаваемых данных, то трогать
> ничего не будет. Только передавать туда-обратно, как до сих пор делал.

Сергей, вам не кажется, что вы нам втираете какую-то дичь? Вот тут http://git.altlinux.org/gears/a/alterator-browser-qt5.git?p=alterator-browser-qt5.git;a=blob;f=alterator-browser-qt/widgets/al_listbox.cc;h=b38f18c35ac1cd2061d531d3d8f5dc713308192e;hb=94a4cdc6d514abfceab3e3e416c00284b9e27e0b#l129 , например, данные не просто передаются туда-обратно, а сохраняются для последующей визуализаии. И здесь можно было бы восстановить ";" из какой-нибудь последовательности других символов.

А ещё лучше было бы взять вот эту часть http://git.altlinux.org/gears/a/alterator-browser-qt5.git?p=alterator-browser-qt5.git;a=blob;f=alterator-browser-qt/widgets/al_listbox.cc;h=b38f18c35ac1cd2061d531d3d8f5dc713308192e;hb=94a4cdc6d514abfceab3e3e416c00284b9e27e0b#l285 (и 280 тоже) и добавить туда поддержку нормального разделения элементов списка, вместо value.split(";", ...). Допустим, сделать <item>...</item> в том же XML, в котором уже сейчас приходит оболочка этих элементов.
Comment 31 Sergey V Turchin 2019-02-01 14:35:43 MSK
(В ответ на комментарий №30)
> Сергей, вам не кажется, что вы нам втираете какую-то дичь?
Вам вотрут любую дичь, вы её тупо передадите дальше и обвините следующего, что он что-то не понял?

> добавить туда поддержку нормального разделения элементов списка,
Хотите ломать -- я не против, ломайте, но без меня.
Comment 32 Антон Мидюков 2019-02-01 14:50:05 MSK
А может стоит посмотреть, как сделано, в шаге инсталлятора "Выбор языка"? Там то всё нормально.

К тому он же умеет менять раскладку ещё и для tty. И есть вот такой баг: https://bugzilla.altlinux.org/show_bug.cgi?id=30932

Может можно адаптировать этот модуль инсталлятора как модуль для альтератора на замену alterator-xkb?
Comment 33 manowar@altlinux.org 2019-02-05 12:43:41 MSK
Давайте, прежде всего, выясним, нужна ли нам поддержка вывода любых символов в виджетах типа список, выпадающий список и т.д.? Если нужна, то логично проапгрейдить формат передачи данных между aterator-lookout и alterator-browser-qt так, чтобы не возникало путаницы.

Если не нужна, то логичнее всего заменить ";" на "," в самом начале, как предлагал Иван. Но это отсрочка до следующего прецедента.
Comment 34 Sergey V Turchin 2019-02-06 11:38:38 MSK
(В ответ на комментарий №33)
> логично
Еще логичнее переписать alterator с нуля, поэтому если вы не собираетесь заниматься разработкой alterator, то и ломать не надо.
 
> Если не нужна, то логичнее всего заменить ";" на "," в самом начале, как
> предлагал Иван.
На входе достаточно привести данные в нужный вид. Т.е. в бэкенде.

> Но это отсрочка до следующего прецедента.
Раз в 10 лет такие преценденты можно не считать проблемой.
Comment 35 AEN 2019-02-18 08:23:36 MSK
(В ответ на комментарий №34)
> (В ответ на комментарий №33)
> > логично
> Еще логичнее переписать alterator с нуля, поэтому если вы не собираетесь
> заниматься разработкой alterator, то и ломать не надо.
> 
> > Если не нужна, то логичнее всего заменить ";" на "," в самом начале, как
> > предлагал Иван.
> На входе достаточно привести данные в нужный вид. Т.е. в бэкенде.
> 
> > Но это отсрочка до следующего прецедента.
> Раз в 10 лет такие преценденты можно не считать проблемой.
Пожалуй.
Как дела с этой багой, коллеги?
Comment 36 manowar@altlinux.org 2019-02-18 13:11:50 MSK
Поясню: тут или нужно тянуть наследство неудачного формата передачи данных дальше (и поставить костыль из комментария #1) или усовершенствовать этот формат вполне конкретным образом. Я вполне определённо высказывался за второй вариант (поскольку в первом моя помощь, очевидно, не требуется). Однако поддержки со стороны мэйнтейнера пакета alterator-browser-qt не получил.
Comment 37 AEN 2019-02-18 13:17:42 MSK
(В ответ на комментарий №36)
> Поясню: тут или нужно тянуть наследство неудачного формата передачи данных
> дальше (и поставить костыль из комментария #1) или усовершенствовать этот
> формат вполне конкретным образом. Я вполне определённо высказывался за второй
> вариант (поскольку в первом моя помощь, очевидно, не требуется). Однако
> поддержки со стороны мэйнтейнера пакета alterator-browser-qt не получил.

Я полагаю, что сейчас, во время бранчей горячки, стоит выбрать первый вариант, пусть и нехороший, и повесить багу о втором. Меня в cc:.
Если это принимается, то прошу Вас поправить. Спасибо.
Comment 38 manowar@altlinux.org 2019-02-18 13:21:15 MSK
Ок
Comment 39 Repository Robot 2019-02-19 01:56:12 MSK
alterator-xkb-3.0-alt3 -> sisyphus:

Mon Feb 18 2019 Paul Wolneykien <manowar@altlinux> 3.0-alt3
- Workaround ALTBUG #35797: replace ';' with ',' in the enumerated
  data (closes: #35797).
Comment 40 AEN 2019-02-19 02:02:12 MSK
Спасибо!
Comment 41 Антон Мидюков 2019-02-19 20:39:51 MSK
Исправлено. Спасибо!
Comment 42 Антон Мидюков 2019-02-28 19:07:44 MSK
Обнаружил, что не всё починилось. Если выбрать раскладку alt+shift, то при следующем запуске alterator будет показывать пустое поле. С остальными раскладками такого не наблюдается.

manowar@, посмотрите, пожалуйста.
Comment 43 manowar@altlinux.org 2019-02-28 19:21:43 MSK
Будем посмотреть…
Comment 44 Антон Мидюков 2019-03-06 10:53:05 MSK
(В ответ на комментарий №42)
> Обнаружил, что не всё починилось. Если выбрать раскладку alt+shift, то при
> следующем запуске alterator будет показывать пустое поле. С остальными
> раскладками такого не наблюдается.
> 
> manowar@, посмотрите, пожалуйста.

Кстати, оно просто не назначается. Потому и поле пустое.
Comment 45 manowar@altlinux.org 2019-03-06 12:27:18 MSK
Хм... Там происходит нечто странное:

# LC_ALL="en" LANGUAGE="en:ru" xkbdatadump

# grep 'alt_shift_toggle' options 
rp:alt_shift_toggle     Alt+Shift

При этом для остальных раскладок имеем правильный дамп:

# grep 'ctrl_alt_toggle' options 
grp:ctrl_alt_toggle     Alt+Ctrl
Comment 46 manowar@altlinux.org 2019-03-06 13:30:01 MSK
Там какой-то страшный разбор XML, написанный на C. И в ходе него, действительно, тепяется буква "g":

(gdb) p *u
value has been optimized out
(gdb) s
do_reset (u=0x4060a0) at xkbdatadump.c:103
103		do_reset(u);
(gdb) p *u
$15 = {el = 0x0, file = 0x405260, mode = 1, layname = 0x409560 "grp", 
  name = 0x412210 "rp:alt_shift_toggle", 
  laydescription = 0x40a410 "Switching to another layout", 
  description = 0x40fca0 "Alt+Shift"}

Наверняка всплыла очередная ошибка с обращением к памяти в связи с переходом на новый компилятор. Если есть силы — помогайте.
Comment 47 manowar@altlinux.org 2019-03-06 15:05:14 MSK
Я собрал его вот с таким патчем:

 static void XMLCALL
 cdata_handler(void *data,const char *buffer, int len)
 {
+    char buf[256];
     user_data *u = data;
 
+    snprintf( buf, len, "%s", buffer );
+    fprintf( stderr, "CDATA: '%s'\n", buf );
+
     if (!u->el) return;

и получил результат:

# LC_ALL="en" LANGUAGE="en:ru" xkbdatadump 2>&1 | grep 'alt_shift'
CDATA: 'rp:alt_shift_toggl'

Получается, что неполные данные передаются уже в callback-функцию cdata_handler() от XML_Parser.
Comment 48 Ivan A. Melnikov 2019-03-06 15:21:50 MSK
(In reply to comment #47)
> Я собрал его вот с таким патчем:
> 
>  static void XMLCALL
>  cdata_handler(void *data,const char *buffer, int len)
>  {
> +    char buf[256];
>      user_data *u = data;
> 
> +    snprintf( buf, len, "%s", buffer );

len+1?

> +    fprintf( stderr, "CDATA: '%s'\n", buf );
> +
>      if (!u->el) return;
> 
> и получил результат:
> 
> # LC_ALL="en" LANGUAGE="en:ru" xkbdatadump 2>&1 | grep 'alt_shift'
> CDATA: 'rp:alt_shift_toggl'
> 
> Получается, что неполные данные передаются уже в callback-функцию
> cdata_handler() от XML_Parser.


А если добавить в /usr/share/X11/xkb/rules/xorg.xml пару пробелов на пару строк выше grp:alt_shift_toggle, прилетает правильно. Очень похоже на баг в expat.
Comment 49 manowar@altlinux.org 2019-03-06 15:23:24 MSK
Да, кстати, забыл написать, что в исходном файле всё в порядке:

# rpm -qf /usr/share/X11/xkb/rules/xorg.xml 
xkeyboard-config-2.25-alt1.noarch

# grep 'alt_shift_toggle' /usr/share/X11/xkb/rules/xorg.xml 
          <name>grp:alt_shift_toggle</name>
Comment 50 manowar@altlinux.org 2019-03-06 15:25:04 MSK
(В ответ на комментарий №48)
> выше grp:alt_shift_toggle, прилетает правильно. Очень похоже на баг в expat.

Ну или в xkbdatadump.c есть ошибки с malloc()/free() которые приводят к ошибкам.
Comment 51 manowar@altlinux.org 2019-03-07 12:45:08 MSK
Мои опасения, похоже, подтвердились:

# LC_ALL="en" LANGUAGE="en:ru" xkbdatadump 2>&1 | grep -e 'MALLOC' -e 'STRDUP' | wc -l
6780

# LC_ALL="en" LANGUAGE="en:ru" xkbdatadump 2>&1 | grep -e 'FREE' | wc -l
4591

Это я решил посчитать, сколько раз память выделяется и сколько освобождается. Вот таким образом:

@@ -35,8 +35,11 @@ typedef struct user_data_struct {
 void 
 do_free(char **item)
 {
-    if (*item) free(*item);
-    *item = 0;
+    if (*item) {
+        free(*item);
+        *item = 0;
+       fprintf (stderr, "FREE\n");
+    }
 }

@@ -47,6 +50,7 @@ do_assign(char **target,const char *destination,int len)
     if (destination && (len > 0))
     {
         *target=malloc(len+1);
+       fprintf (stderr, "MALLOC\n");

@@ -73,6 +77,7 @@ element_start(void *data, const char *el, const char **attr)
     user_data *u = data;
 
     u->el = strdup(el);
+    fprintf (stderr, "STRDUP\n");


Наверное, можно было через strace тоже.
Comment 52 Ivan A. Melnikov 2019-03-07 12:52:14 MSK
Ну то, что оно течёт, видно и невооруженным валгриндом:

$ valgrind  --leak-check=full xkbdatadump
==7351== Memcheck, a memory error detector
==7351== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==7351== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==7351== Command: xkbdatadump
==7351==
==7351==
==7351== HEAP SUMMARY:
==7351==     in use at exit: 21,300 bytes in 2,190 blocks
==7351==   total heap usage: 7,018 allocs, 4,828 frees, 135,302 bytes allocated
==7351==
==7351== 143 (56 direct, 87 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 4
==7351==    at 0x48399A2: calloc (vg_replace_malloc.c:762)
==7351==    by 0x401237: main (xkbdatadump.c:161)
==7351==
==7351== 793 bytes in 92 blocks are definitely lost in loss record 3 of 4
==7351==    at 0x4837753: malloc (vg_replace_malloc.c:309)
==7351==    by 0x4B399C9: strdup (strdup.c:42)
==7351==    by 0x4014A3: element_start (xkbdatadump.c:75)
==7351==    by 0x4890775: doContent (xmlparse.c:2922)
==7351==    by 0x4890F1B: contentProcessor (xmlparse.c:2584)
==7351==    by 0x4892893: doProlog (xmlparse.c:4604)
==7351==    by 0x4892FF7: prologProcessor (xmlparse.c:4321)
==7351==    by 0x4895407: XML_ParseBuffer (xmlparse.c:2081)
==7351==    by 0x40129E: main (xkbdatadump.c:173)
==7351==
==7351== 20,364 bytes in 2,093 blocks are definitely lost in loss record 4 of 4
==7351==    at 0x4837753: malloc (vg_replace_malloc.c:309)
==7351==    by 0x4B399C9: strdup (strdup.c:42)
==7351==    by 0x4014A3: element_start (xkbdatadump.c:75)
==7351==    by 0x4890775: doContent (xmlparse.c:2922)
==7351==    by 0x4890F1B: contentProcessor (xmlparse.c:2584)
==7351==    by 0x4895407: XML_ParseBuffer (xmlparse.c:2081)
==7351==    by 0x40129E: main (xkbdatadump.c:173)
==7351==
==7351== LEAK SUMMARY:
==7351==    definitely lost: 21,213 bytes in 2,186 blocks
==7351==    indirectly lost: 87 bytes in 4 blocks
==7351==      possibly lost: 0 bytes in 0 blocks
==7351==    still reachable: 0 bytes in 0 blocks
==7351==         suppressed: 0 bytes in 0 blocks
==7351==
==7351== For counts of detected and suppressed errors, rerun with: -v
==7351== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)


Но это не выглядит, как проблема. Да, неаккуратно, но для утилиты, которая один раз запускается и быстро выходит это не критично. В то же время никаких double free, а также доступа к невыделенной или неинициализированной памяти валгринд не обнаруживает.
Comment 53 Ivan A. Melnikov 2019-03-07 13:05:08 MSK
Посмотрел ещё раз, с патчем, очень похожем на comment #47, но с len+1 в strncpy, так что оно не обрезает последний символ:

diff --git a/alterator-xkb/tools/xkbdatadump/xkbdatadump.c b/alterator-xkb/tools/xkbdatadump/xkbdatadump.c
index 43b3bd1..32d4448 100644
--- a/alterator-xkb/tools/xkbdatadump/xkbdatadump.c
+++ b/alterator-xkb/tools/xkbdatadump/xkbdatadump.c
@@ -132,8 +132,12 @@ element_stop(void *data, const char *el)
 static void XMLCALL
 cdata_handler(void *data,const char *buffer, int len)
 {
+    char buf[256];
     user_data *u = data;

+    snprintf( buf, len+1, "%s", buffer );
+    fprintf( stderr, "CDATA: '%s'\n", buf );
+
     if (!u->el) return;

     if (!strcmp(u->el,"name"))



Если посмотреть внимательно, имеем такой кусочек:
CDATA: '          '
CDATA: 'g'
CDATA: 'rp:alt_shift_toggle'
CDATA: '


Это валидное поведение expat и вообще любого xml-парсера: он может вызывать character data handler сколько угодно раз хоть для каждого символа в отдельности. xkbdatadump, очевидно, такого не ожидает, хотя должен.
Comment 54 manowar@altlinux.org 2019-03-07 13:40:21 MSK
(В ответ на комментарий №53)
> Если посмотреть внимательно, имеем такой кусочек:
> CDATA: '          '
> CDATA: 'g'
> CDATA: 'rp:alt_shift_toggle'
> CDATA: '
> 
> 
> Это валидное поведение expat и вообще любого xml-парсера: он может вызывать
> character data handler сколько угодно раз хоть для каждого символа в
> отдельности. xkbdatadump, очевидно, такого не ожидает, хотя должен.

О, теперь понятно. Спасибо.
Comment 55 Ivan A. Melnikov 2019-03-07 13:54:23 MSK
(In reply to comment #53)
> Это валидное поведение expat и вообще любого xml-парсера: он может вызывать
> character data handler сколько угодно раз хоть для каждого символа в
> отдельности. xkbdatadump, очевидно, такого не ожидает, хотя должен.

Посмотрел на код -- получается, xkbdatadump надо значительно так переписать. Могу этим заняться на выходных.
Comment 56 manowar@altlinux.org 2019-03-07 13:57:04 MSK
(В ответ на комментарий №55)
> (In reply to comment #53)
> > Это валидное поведение expat и вообще любого xml-парсера: он может вызывать
> > character data handler сколько угодно раз хоть для каждого символа в
> > отдельности. xkbdatadump, очевидно, такого не ожидает, хотя должен.
> 
> Посмотрел на код -- получается, xkbdatadump надо значительно так переписать.
> Могу этим заняться на выходных.

Ну, я вроде начал уже. Решил переписать на статику вообще. Если нужно будет, потом сделаем realloc(). Но пока кажется, что это лишнее.
Comment 57 AEN 2019-05-26 21:33:51 MSK
(В ответ на комментарий №56)
> (В ответ на комментарий №55)
> > (In reply to comment #53)
> > > Это валидное поведение expat и вообще любого xml-парсера: он может вызывать
> > > character data handler сколько угодно раз хоть для каждого символа в
> > > отдельности. xkbdatadump, очевидно, такого не ожидает, хотя должен.
> > 
> > Посмотрел на код -- получается, xkbdatadump надо значительно так переписать.
> > Могу этим заняться на выходных.
> 
> Ну, я вроде начал уже. Решил переписать на статику вообще. Если нужно будет,
> потом сделаем realloc(). Но пока кажется, что это лишнее.

Как дела?
Выходные заканчиваются. :)
Comment 58 manowar@altlinux.org 2019-05-27 20:50:23 MSK
Точно. Я просто забыл собрать в тот раз. Отправил test only в Сизиф сейчас: http://git.altlinux.org/tasks/230470/ .
Comment 59 AEN 2019-05-27 20:55:10 MSK
(В ответ на комментарий №58)
> Точно. Я просто забыл собрать в тот раз. Отправил test only в Сизиф сейчас:
> http://git.altlinux.org/tasks/230470/ .

Failed
Comment 60 manowar@altlinux.org 2019-05-27 21:10:51 MSK
Да, нужно доделать. Завтра займусь этим.
Comment 61 manowar@altlinux.org 2019-05-28 16:16:55 MSK
TESTED. Я проверил на относительно свежем Сизифе и там оно работает. Коммитим / закрываем?
Comment 62 Антон Мидюков 2019-05-28 16:28:00 MSK
(В ответ на комментарий №61)
> TESTED. Я проверил на относительно свежем Сизифе и там оно работает. Коммитим /
> закрываем?

Я сейчас проверю.
Comment 63 Антон Мидюков 2019-05-28 16:41:47 MSK
(В ответ на комментарий №62)
> (В ответ на комментарий №61)
> > TESTED. Я проверил на относительно свежем Сизифе и там оно работает. Коммитим /
> > закрываем?
> 
> Я сейчас проверю.

Коммитьте, закрывайте. Спасибо!
Comment 64 AEN 2019-05-28 17:03:44 MSK
Спасибо
Comment 65 manowar@altlinux.org 2019-05-28 17:11:44 MSK
*** Bug 36800 has been marked as a duplicate of this bug. ***