История однострочных багов

[dimkagfx]

Never forget

[Snowly]

Не всем думаю будет понятно, да и комит не в самом верху страницы.

-BUMBLEBEEVERSION=1.4.31
+BUMBLEBEEVERSION=1.4.32

@@ -348,7 +348,7 @@ case "$DISTRO" in
   ln -s /usr/lib/mesa/ld.so.conf /etc/alternatives/gl_conf
   rm -rf /etc/alternatives/xorg_extra_modules
   rm -rf /etc/alternatives/xorg_extra_modules-bumblebee
-  rm -rf /usr /lib/nvidia-current/xorg/xorg

Пробел после /usr. Меня тогда пронесло, не успел обновится.

[nomit]

О да, а я тогда голову сломал чего это у меня система сломалась. И понять не мог, на вид как будто ядро не видит :)

[j_wayne]

ИМХО если бы не было дублирования префикса /usr/lib, вероятность такой опечатки была бы меньше… DRY

[SovGVD]

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

[JIghtuse]

Вы спрашиваете, чтобы потроллить или просто чтобы в поисковик не ходить? Каким образом связана открытость/закрытость проекта и его тестирование?
Вот вам пара open-source продуктов, которые тестируют так, как пожалуй ничто в мире.
SQLite
Chromium

[linoleum]

>>> Android <...> То есть один из передаваемых в функцию параметров вообще не используется.
Очень странно, что такой баг возник, учитывая, что большинство (если не все) IDE на такие вещи истерично ругаются.

[ilmirus]

Не только IDE, но и компиляторы.

[eyeless_watcher]

Описание этого бага выдаёт автора (точнее, что тот ни в зуб ногой в том, о чём пишет).
Параметр v и не должен возвращаться, это то значение, которым заполняется память по заданному указателю. Возвращается всё правильно в обоих случаях, баг был в том, что память всегда обнуляется.

[dalamber]

Например, «замемсеттить» RGB24 буфер значением 255, чтобы получить белую канву. Но это я так, к слову — обычно канва нужна чёрная :)

[splav_asv]

В релизе — да, а для отладки забивают 0xCC или 0xDEADBEEF или еще чем-то в этом роде. Так что этот баг слегка затруднял отладку. Промахнулся на один уровень.

[splav_asv]

Это был просто пример того, что не всегда пишут memset 0, иначе можно ыбло не объявлять bzero устаревшей. Для большинстве платфор никак нельзя memset'ами писать шаблоном шире одного байта, на сколько я знаю. Но на Mac OS X есть в libc функции memset_pattern[4,8,16], относящиеся к семейству memset.

[QtRoS]

Тоже хотел написать про это.

Но раз уж Вы опередили, то вот другой вопрос — неужели memset там реализован через цикл? Небольшое гугление говорит, что

The underlying implementation might be in assembler specific to that architecture to take advantage of whatever its native properties are. It might also have internal special cases to handle the case of zeroing (versus setting some other value).

[Aux]

Потому что vim. В исходниках Android очень много такой фигни.

[ximaera]

Использование vim какбэ не отменяет GCC-флаги -Wall -Wextra -Werror. Если человеку наплевать на предупреждения компилятора, то ему и на IDE будет наплевать.

[Lol4t0]

Работает, только если при компиляции у вас не генерится еще 1000 'unused parameter' warning-ов. Достаточно одного человека в команде, который допускает в коде предупреждения, чтобы проанализировать лог компилятора стало сложно.

[ximaera]

Если у вас есть такой человек, то vim и warning'и — это ещё не самая большая из ваших бед. Устранять надо корень проблемы, а не следствия.

[Lol4t0]

К сожалению, в больших проектах такое бывает. Человек может банально собирать в другой конфигурации, в которой переменные используются.

[ximaera]

__attribute__((__unused__))?

[Lol4t0]

Для этого надо хотя бы знать, что переменная не используется.

если есть код

int x = bar();
#ifdef EXT_LOG
ext_log(x);
#endif

И вы всегда собираете с EXT_LOG defined, а ваш товарищ — всегда с EXT_LOG undefined, то тут и возникает проблема.

[ximaera]

#ifndef EXT_LOG
__attribute__((__unused__)
#endif
int x = bar();

?

Хотя тогда уже проще само существование переменной x поставить в зависимость от EXT_LOG.

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

[Lol4t0]

Да вы поймите, что товарищ, который код правит — не получает предупреждения и потому не замечает ошибки. А вы тоже не хотите лезть со своими правками в чужой код.

[ximaera]

Практика code review у вас не распространена? Кроме того, можно попросить коллегу внести эти изменения в код, потому что это упростит вам работу и потенциально позволит избежать проблем в релизе. И вообще, не вижу ничего плохого в том, чтобы предложить другому человеку патч, тем более столь тривиальный и лаконичный.

[burjui]

_{С этими словами он бросил в сумку АКСУ и несколько магазинов к нему, после чего последовал к выходу.}

[Aux]

Регулярно собирая open source софт из исходников, могу сказать, что ВЕСЬ софт написанный в vim просто пестрит ворнингами.

В Android штука интересная в том, что большинство кода написано на Java, а компилятор не линтует код. При этом любая IDE для Java гоняет линт автоматом и сразу бьёт по рукам. Не сильно, но надоедливо. Люди, привыкшие к консоли линт не юзают в принципе, а потом их код зарастает тоннами навоза.

IDE очень сильно дисциплинирует и пресекает множество мелких косяков на корню. За использование текстовых редакторов в программировании надо выгонять из профессии. Это вам не посты в бложики писать.

[ximaera]

Уважаемый, да вам с таким подходом в Госдуму надо идти вырабатывать требования к профессиональным программистам. Серьёзно, я даже не знаю, что тут ещё можно сказать

Если некто не умеет пользоваться неким инструментарием, это ещё не повод навязывать всем окружающим своё мнение. Казалось бы, это банальность, да?

[Aux]

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

[ximaera]

Плохая аналогия подобна котёнку с дверцей. Это раз.

Если ваш плотник достигнет феноменальной выработки при минимальных затратах, забивая гвозди микроскопом, то последнее, что вы должны делать — отбирать у него микроскоп и совать ему свой проклятый хлипкий молоток с зазубринами и неудобной ручкой. Оценивайте результаты, а не инструменты, чтобы не выяснилось как-нибудь, что вы ненароком половину разработчиков ядра Linux из профессии выгнали. Это два.

[Aux]

Работа над ядром Linux не такая уж и феноменальная работа. Мелкий патчик и в ноутпаде наклепать можно, и в общей архитектуре ядра для этого разбираться не надо.

[ximaera]

То есть, на ваш взгляд, работа над ядром сводится к наклёпыванию мелких патчиков?

[ilmirus]

>Регулярно собирая open source софт из исходников, могу сказать, что ВЕСЬ >софт написанный в vim просто пестрит ворнингами.

Я вот специально скачал последнюю версию GCC из транка и собрал без ключа --disable-bootstrap. Предупреждения:
1) format not a string literal and no format arguments
2) unknown conversion type character ‘X’ in format
3) too many arguments for format

Все они -Wformat* и не ломают бутстрап, ибо признаны некритичными. Однако стоит объявить переменную и не использовать ее, сразу же ошибка бутстрапа.

Так ли вы уверены, что код, написанный без IDE плох?

>IDE очень сильно дисциплинирует и пресекает множество мелких косяков на корню

Посоветуете IDE, которая понимает ВЕСЬ GCC, со всей магией?

[Aux]

К сожалению, я очень давно не пишу на C/C++ и ничего вам посоветовать не могу.

[JIghtuse]

Вот вам код из статьи в Vim с плагином YouCompleteMe.

Скрытый текст

[WraithOW]

Люди, привыкшие к консоли линт не юзают в принципе

Из консоли — это "javac *"? Мсье знает толк в несвежих извращениях.

любая IDE для Java гоняет линт автоматом

1) Не любая.
2) Инспекции для начала надо бы и настроить.
Так что не панацея.

[ironsnake]

Странно. Пишу код в Vim. И этот самый Vim выдает мне и ворнинги, и ошибки и все-все-все. Комплита нет, да, потому что не настраивал, ибо не люблю.
UPD. Да, gdb я к нему тоже прикручивал ради фана.

[imwode]

>Debian OpenSSL
При использовании нормального IDE это невозможно
А вот поделки типа IWARM — там еще и не такой хрени понаделаешь

[mayorovp]

И что же нормальная IDE тут сделает-то?

[imwode]

высветит строку как коммент

[mayorovp]

Насколько я понял, строка была закоментирована намеренно, по причине "Don't add uninitialised data."

[imwode]

… во время отладки, а в релизе забыл раскомментить, потому что смотрелось ок

хотя кто знает

[allter]

Вообще-то закомментировали намеренно, т.к. кому-то пришла в голову (или родилась на совещании в АНБ) гениальная мысль пройтись по исходникам профилировщиком valgrind, который высветил «использование не инициализированных данных», которое было бы ошибкой в любом проекте, кроме ГПСЧ, где это внезапно оказалось фичей.

[amarao]

Да, и был большой срач между дебиановцами и openssl'щиками. Потому что собирать энтропию из неинциализированной памяти — та ещё эвристика.

[khim]

Там эта переменная частично инициализирована. Они берут большой буфер и начинают класть в него всё подряд: информацию о процессах, что-то из /dev/random, etc. Но до конца буфер так и не заполняют, в результате чего часть буфера так и остаётся неинициализированной. После чего весь буфер скармливается в ГПСЧ.

Так что неинициализированные данные — это далеко не основной источник энтропии для ГПСЧ — но основной источник ругани valgrind'а. Уж сколько раз твердили миру, что все автоматические тулы — это лишь помощники, для интерпретации их сообщений голова нужна.

[amarao]

А что мешало им этот буффер занулить и складывать туда всё остальное? Срач-то был по поводу того, что «не-иницализация буффера» — это дурная-дурная практика.

[alexac]

Вообще-то эта строка вообще является undefined behaviour, gcc и clang могут такую строку вовсе выкинуть из исходников, перенести в другое место с нарушением сайд эффектов и натворить прочие радости.

[udachnik]

Я целую минуту всматривался в строчку
if (getuid() != 0 && geteuid() == 0)
пытаясь понять ее смысл. Потом заметил.

[ploop]

Вот что значит неудачный идентификатор…

[Borz]

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

[WerewolfPrankster]

Да нет. Тут беда в том, что имена функций различаются в одну букву. Я тоже тупил секунд 10. А не знал бы — и не заметил бы.

[Borz]

эм. а разве разница не в том, что после «geteuid» не было скобок?

[ploop]

Ну это как-бы явный баг, описанный в посте.
В общем проблема в другом: getuid() и geteuid() не мудрено попутать в любом месте кода, и IDE ничем не поможет, так как функции однотипные. И тупить можно не один час, вылавливая такой баг.

[amarao]

Для любого юниксоида разница между euid и uid понятна, и путать их — всё равно, что путать slip() и sleep().

[ploop]

get_uid()/get_euid(), getUid()/getEuid() в зависимости от принятых стандартов, и ещё масса вариантов.
Дело не в разнице или её знании/не знаии, дело просто в зрительном восприятии.

[amarao]

Ну так функций, которые на одну букву отличаются в названии очень много (vnsprintf vs vnprintf). Это вполне стандартная практика в Си, и с учётом того внимания, которое надо проявлять при работе с памятью, внимание к имени функции просто неощутимо.

[Lol4t0]

Стандартная-то она может и стандартная, но она ужасна.

[aryeh]

Минимум 2 из 5 ошибок были бы найдены самым простым статическим анализатором кода.

[Andrey2008]

У меня тут таких ляпов.... Другое дело, что это просто ошибки, а не известные epic fail.
Используйте варнинги компилятора и статический анализ. Часто помогает.

[m08pvv]

Писать с упором на MISRA-C, собирать с -Wall -Wextra -Wpedantic — да, некоторые так делают (и даже больше).

[valplo]

А как же the most expencive overbar in history?

[poige]

Баг у xserver вообще некритичный, ни разу. Более того, эта проверка ещё и не нужна, ибо лучше не проверять на рутовость, а просто обламываться там, где не хватает прав, желательно с адекватной диагностикой. Это гораздо более гибкий подход, ведь иногда пользователь может входить в нужную группу/обладать достаточными привилегиями, но при этом EUID не будет root'овым.

[mayorovp]

Этот код не имеет отношения к проверке прав. Это — защита на тот случай, если админ поставил программе флаг setuid. Смысл защиты в том, что в режиме setuid программа может работать так, как была настроена — но не может быть перенастроена заного. Лично я не понимаю, зачем вообще так делать — но, видимо, юзкейсы были.

[poige]

О чём я и говорю — при -configure сервер запишет новый конфиг-файл. Проверка почему-то предполагает, что не-root-пользователь этого сделать не должен, но, что если именно этот пользователь является владельцем конфиг-файла, и, соответственно, по-логике вещей, должен был бы суметь воспользоваться этой опцией. Типичный случай, когда assumption is the mother of all f*ckups.

[mayorovp]

Вглядитесь еще раз в эту строчку: if (getuid() != 0 && geteuid() == 0) {
Если админ не выставлял программе флага setuid — любой пользователь без проблем настроит свой личный конфиг.

[poige]

Что ж так упорно понимать не хотите?

www.wikihow.com/Configure-X11-in-Linux:

A new file has been created in /etc/X11/ called xorg.conf

+ man:

-configure
When this option is specified, the Xorg server loads all video driver modules, probes for available hardware, and writes out an initial xorg.conf(5) file based on what was detected. This option currently has some problems on some platforms, but in most cases it is a good way to bootstrap the configuration process. This option is only available when the server is run as root (i.e, with real-uid 0).

Суть моего коммента в том, что пользователи могут что-то вполне административно-легально (права-членства в группах, или собственность) делать вещи, которые обычно может делать root. Данная проверка этой возможности его лишает. Именно поэтому нужно «… не проверять на рутовость, а просто обламываться там, где не хватает прав, желательно с адекватной диагностикой. …».

[mayorovp]

Данная проверка — не лишает! Что же вы вчитаться в булево условие не желаете так упорно?..

PS и да, хотелось бы посмотреть на пользователя, имеющего права на загрузку драйверов…

[poige]

То есть выдержка из man'а, где просто по-английски написано, что -configure можно выполнить только будучи root'ом, не смущает. Я даже курсивом выделил.

> PS и да, хотелось бы посмотреть на пользователя, имеющего права на загрузку драйверов…

Наслаждайтесь, чё…

[mayorovp]

Меня смущает не выдержка из мана, а тот факт, что обвиняют совершенно постороннюю проверку.

PS спасибо.

[poige]

Пожалуйста. А теперь попробуйте подумать логически о том, что я выше написал.

-configure даёт возможность получить текстовое представление рабочего конфига сервера. Автоматически. root-пользователь просто запускает с этим ключом сервер, тот сохраняет рабочий конфиг.

Проверка «если root не настоящий» (а только SETUID'ный) -configure делать запрещает

ErrorF("The '-configure' option can only be used by root.\n");

— ну а как же, вдруг Вася Пупкин изуродует более культурный конфиг, более автоматическим.

А теперь — что если Вася Пупкин как раз и есть владелец /etc/X11/xorg.conf? Почему Вася, не должен смочь записать новый конфиг?

[mayorovp]

Потому что для этого нужна более сложная проверка. Зачем тратить силы на те юзкейсы, которыми никто не пользуется?

Васе же Пупкину никто не запрещает сделать так (если у него есть права грузить драйвера — то и куда исполнимый файл записать найдется):

cp /path/to/Xorg ./Xorg
chmod +x ./Xorg
./Xorg -configure

И еще есть тот вариант, в котором рут снимает флаг setuid с файла Xorg — и Васе Пупкину ничего не надо никуда копировать.

[poige]

Потому что для этого нужна более сложная проверка. Зачем тратить силы на те юзкейсы, которыми никто не пользуется?

Во-1-х, потому что идеологически правильнее проверять должна операционная система. Например, из-за появления тех же capabilites в Linux, кучу таких проверок пришлось найти, и выбросить…
Во-2-х, «никто не пользуется» — да-да. Расскажите UNIX-администраторам, что они не должны больше пользоваться стандартной моделью прав доступа, а должны каждому Васе сразу выдавать права root.

если у него есть права грузить драйвера — то и куда исполнимый файл записать найдется):

Если SETUID на данной платформе был реально необходим для штатной работы X-сервера, то после копирования, файл X превратится в тыкву.

(Думаю теперь можно поставить точку в этом диалоге.)

[mayorovp]

Вы сами себе противоречите. Все-таки, есть у Васи Пупкина необходимые права — или нет?

Во-1-х, потому что идеологически правильнее проверять должна операционная система. Например, из-за появления тех же capabilites в Linux, кучу таких проверок пришлось найти, и выбросить…

Скажите мне, КАК должна операционная система проверять права Васи Пупкина, если программа работает под euid 0? Проблема именно в том, что setuid root выключает все проверки со стороны операционной системы — и прикладная программа должна проводить их самостоятельно.

Конечно же, можно вместо сообщения об ошибке написать следующее:

if (getuid() != 0 && geteuid() == 0)
    setuid(getuid());

Это вернет проверку прав на уровень ОС — но такое действие эквивалентно флага setuid у файла. То есть возвращаемся к варианту

Если SETUID на данной платформе был реально необходим для штатной работы X-сервера, то после копирования, файл X превратится в тыкву.

PS хорошие админы UNIX давно уже усвоили, что setuid — зло. Поэтому хорошим админам эта проверка совсем не мешает.

(Думаю, да, теперь можно и точку поставить)

[poige]

Скажите мне, КАК должна операционная система проверять права Васи Пупкина, если программа работает под euid 0?

В UNIX это называется Saved UID. На этом позволю себе таки окончательно закончить сеанс вашего бесплатного образования.

[IamKarlson]

Автор, а почему вы не указываете что это перевод?

[mayorovp]

Потому что он никогда не указывает этого.

[kolpeex]

Забавно, как раз сегодня отловил такой баг:
$a = true;
$b = false;
$c = $a and $b;
var_dump($c); // bool (true)

[valplo]

Прошу прощения за мое невежество, но это на каком языке?

[resurtm]

PHP. Дополнение:

$a = true;
$b = false;
$c = $a and $b;
$d = $a && $b;
var_dump($c); // bool (true)
var_dump($d); // bool (false)

[galaxy]

В чем баг?

[bohdan-shulha]

Это не баг. Это, немного неожиданное для рядового программиста, нормальное поведение.

$c = $a and $b;

читается как

($c = $a) and $b;

ua1.php.net/operators.precedence

[valplo]

А я-то думал, чего все так не любят PHP?

[VolCh]

Вообще это фича Perl.

[newcommer]

Да и Perl, кажется, не все любят :)

[poige]

Это, немного неожиданное для рядового программиста, нормальное поведение.

— “Normal everyday psychosis” © Bruce Almighty

[kolpeex]

Да, в документации описано. Но все же…

[zyss]

(уже написали)