LINUX.ORG.RU

Линус о предупреждениях компилятора


0

0

В ответ на патч, который прячет предупреждения компилятора (GCC) Линус Торвальдс написал следующее: "Пожалуйста, НЕ исправляйте без его полного понимания код, из-за которого компилятор выдаёт предупреждения. Это прямой путь к добавлению новых ошибок, а не исправлению старых. Поэтому, пожалуйста, пожалуйста, пожалуйста, осознайте, что компилятор - это тупое существо, и исправление ошибок без их понимания - это плохо".

"В этом случае, любой, кто потратит пять секунд на изучение кода, должен осознать, что предупреждение компилятора - это просто иной способ высказать мысль о том, что автор принимал тяжёлые наркотики, и что эти предупреждения следует оставить! Потому что этот код должен приводить к ругани компилятора. Это жирные предупреждения о некомпетентности!"

>>> Подробности

Ответ на: комментарий от Teak

Сто баксов... Хм, тогда надо будет засунуть в ядро побольше конструкций типа:

#define M if ( a =\
= b ) printf("Bla bla\n");

Деньги надо зарабатывать потом :))
Этож сколько проектов то сейчас на Си есть... Даже если брать только релизы ядра... Богатым станете :)

*=  ->  *:=     или    *=  ->  :*=    Как красивее?

mky ★★★★★
()
Ответ на: комментарий от sS

> Сравни читаемость 2-х вариантов из реального кода

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

anonymous
()
Ответ на: комментарий от anonymous

Поправочка

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

Ты имеешь ввиду что в конце обоих кусков кода не стоит

return rc;

?

Он просто не попал в вырезанный кусок всего навсего потому как не имеет отношения к теме.

sS ★★★★★
()
Ответ на: комментарий от anonymous

>.... почти во всех прог самого разного пошиба ....

Неправда. Полно софта, который собирается без warnings.

> М.б. потому, что в большинстве случаев их просто невозможно избежать? ;)

М.б., потому, что авторы не _стремятся_ это сделать, и часть пишут на авось?

stellar
()
Ответ на: комментарий от stellar

>часть пишут на авось?

часть кода, а то и весь код

stellar
()
Ответ на: комментарий от anonymous

>мне (1) как--то милее 
man fork, уважаемый 

pid_t pid = fork(); 

if (pid == 0)
{
    /* child */
}
else if (pid > 0)
{
    /* parent */
}
else
{
    fprintf(stderr, "Can't fork: %s", strerror(errno)); 
} 

stellar
()
Ответ на: комментарий от stellar

Поправочка

>М.б., потому, что авторы не _стремятся_ это сделать, и часть пишут на авось?

Оно не всегда так, в том числе из за читаемости кода и много чего еще.

Я например уже многие годы держу у себя в одном из проектов 
вот такой вот варнинг. 

DataFrame.cpp:285: warning: cast from pointer to integer of different size

На вот таком вот коде:

FXbool DataFrame::isDataValid() {
    return (FXbool)root_item; 
}

Вместо того чтобы написать:

FXbool DataFrame::isDataValid() {
    if(root_item)
      return true;
    else
      return false;
}

Потому как мне 1-й вариант понятнее ;)


sS ★★★★★
()
Ответ на: комментарий от Annymous

ага 11 лет опыта на олимпиад на сях.

Не вижу проблем с читаемостью. Особенно при нормально настроеной подсветке, которая отдельно подсвечивает присваивание внутри выражений.

Вообще то тут речь не о читаемости, а о предупреждениях.

sergej ★★★★★
()
Ответ на: Поправочка от sS

> Ты имеешь ввиду что в конце обоих кусков кода не стоит return rc; ?

Нет. Я имею ввиду, что сам код работает по-разному при одних и тех же условиях. Посмотри внимательнее :)

anonymous
()
Ответ на: комментарий от stellar

> Неправда. Полно софта, который собирается без warnings.

Просто их скрыли, не став включать -W -Wall -pedantic

А ядро и его модули, имхо, нужно вообще с -ansi -W -Wall -Werror собирать, достали глюки!

anonymous
()
Ответ на: комментарий от anonymous

>Нет. Я имею ввиду, что сам код работает по-разному при одних и тех же условиях. Посмотри внимательнее :)

При каких ?

sS ★★★★★
()
Ответ на: комментарий от anonymous

>return (root_item?true:false);

Те же яйца что и 2 токо сбоку...только еще более нечитабельно =)

sS ★★★★★
()
Ответ на: Поправочка от sS

> DataFrame.cpp:285: warning: cast from pointer to integer of different size
...
> return (FXbool)root_item;

Кстати, есть вероятность, что получишь ошибку в алгоритме: если занимаемое в памяти место root_item-ом больше, чем переменной, типа bool, то может возникнуть ситуация, при которой вернется значение в нулями в младших разрядах и неким значением в старших разрядах (справедливо для x86), а в анализ будет производиться по размеру bool. Соостветственно, программа увидит только младшие разряды (который содержат 0) и пойдет по ветке, когда возвращается значение false.

Еще есть вероятность получить seg. fault: если занимаемое в памяти место root_item-ом меньше, чем переменной, типа bool, то при передаче данных размерности bool из адреса root_item программа может выйти за пределы выделенной памяти, на что ядро отреагирует незамедлительно.
Так же при этом возможно повторение первого варианта, только в обратной форме - по адресу root_item содержатся нули (переменная не определена, насколько я понял), но соседняя область памяти (которая захватится размерностью типа bool) содержит что-то отличное от 0. Таким образом, из программы вернется значение true, хотя должно вернуться false.

anonymous
()
Ответ на: комментарий от sS

> При каких ?

Немного перепишем первый вариант: --- if (rc) goto err_pictsetting; ... err_pictsetting: video_device_remove_file(vdev, &class_device_attr_model); err_model: video_device_remove_file(vdev, &class_device_attr_stream_id); ---

Где во втором варианте возможен вызов двух функций video_device_remove_file подряд?

anonymous
()
Ответ на: комментарий от anonymous

Извиняюсь за форматирование:

> При каких ?

Немного перепишем первый вариант:
---
if (rc) goto err_pictsetting;
...
err_pictsetting: video_device_remove_file(vdev, &class_device_attr_model);
err_model: video_device_remove_file(vdev, &class_device_attr_stream_id);
---

Где во втором варианте возможен вызов двух функций video_device_remove_file подряд?

anonymous
()
Ответ на: комментарий от sS

>>Нет. Я имею ввиду, что сам код работает по-разному при одних и тех же условиях. Посмотри внимательнее :) > При каких ?

Вы видимо инеете ввиду отсутствие еще одной video_device_remove_file(vdev, &class_device_attr_stream_id); ?

Ну дык это понятно же что опечатка (сама хункция сильно длиннее приведённого куска и это издержки cut&paste) ;)

sS ★★★★★
()
Ответ на: комментарий от anonymous

>Кстати, есть вероятность, что получишь ошибку в алгоритме: если занимаемое в памяти место root_item-ом больше, чем переменной, типа bool, то может возникнуть ситуация, при которой вернется значение в нулями в младших разрядах и неким значением в старших разрядах (справедливо для x86), а в анализ будет производиться по размеру bool. Соостветственно, программа увидит только младшие разряды (который содержат 0) и пойдет по ветке, когда возвращается значение false.

В теории такая вероятность есть.

sS ★★★★★
()
Ответ на: комментарий от sS

Зачем писать так

     if(!(rc = video_device_create_file(vdev, &class_device_attr_stream_id))) { 
         if(!(rc = video_device_create_file(vdev, &class_device_attr_model))) {
            if((rc = video_device_create_file(vdev, &class_device_attr_pictsetting))) 
                video_device_remove_file(vdev, &class_device_attr_model);
         } else {
            video_device_remove_file(vdev, &class_device_attr_stream_id);
         }
     }

если можно так

rc = video_device_create_file(vdev, &class_device_attr_stream_id);
if(!rc) {
     rc = video_device_create_file(vdev, &class_device_attr_model);
     if(!rc) {
         rc = video_device_create_file(vdev, &class_device_attr_pictsetting);
         if(rc) 
             video_device_remove_file(vdev, &class_device_attr_model);
         } else {
             video_device_remove_file(vdev, &class_device_attr_stream_id);
     }
}
?

k_andy ★★★
()
Ответ на: комментарий от sS

>>return root_item != NULL;

>Вот такой вариант точнее пожалуй будет.

Есть шанс, что вы наконец-то избавитесь от warning'а? :)

k_andy ★★★
()
Ответ на: комментарий от k_andy

>Зачем писать так

IMHO читабельнее ;)

sS ★★★★★
()
Ответ на: комментарий от k_andy

> Есть шанс, что вы наконец-то избавитесь от warning'а? :)

Без него будет немного грустно ;)

Он был единственным и показывал что ~50% сборки завершено.

/me вместо него добавил #warning 50% complete ...

sS ★★★★★
()
Ответ на: комментарий от anonymous

>Назло Линусу ? :D

У Линуса своих варнингов хватает ну и кода у него поболее будет (раз в 15)

sS ★★★★★
()
Ответ на: комментарий от anonymous

>Просто их скрыли, не став включать -W -Wall -pedantic

Кое-где и скрыли. Тем не менее, куча софта собирается даже с -Wall.

>А ядро и его модули, имхо, нужно вообще с -ansi -W -Wall -Werror собирать, достали глюки!

Боюсь, оно не соберется :)

stellar
()
Ответ на: комментарий от Quasar

>чтобы некоторые кодеры задумывались. Полезная тема.

Точнее былокодеры..

anonymous
()
Ответ на: комментарий от stellar

>Боюсь, оно не соберется :)

На 100% ;)

Ну например любимые кернельные фичи типа

enum  {
type_one,
type_two,
};

пойдут лесом ;)

sS ★★★★★
()
Ответ на: Поправочка от sS

> return (FXbool)root_item;

Не будет работать на тех архитектурах, где sizeof(FXbool) != sizeof(type of root_item).

Точнее, работать оно будет; вот только результат непредсказуем. И из-за подобного стиля куча софта как раз нормально и не портируется.

stellar
()
Ответ на: комментарий от stellar

Ну наверняка ничего путного.

anonymous
()

по тому же файлу (serverworks.c) , предлагаю обсудить вот такой код:

                     if ((dmaspeed & 0x20) == 0x20)
                             dmaspeed = XFER_MW_DMA_2;
                     else if ((dmaspeed & 0x21) == 0x21)
                             dmaspeed = XFER_MW_DMA_1;
                     else if ((dmaspeed & 0x77) == 0x77)
                             dmaspeed = XFER_MW_DMA_0;
                     else
                             goto dma_pio;


как вы думаете он работает?

anonymous
()
Ответ на: комментарий от anonymous

В таком порядке будет.
соответствено маски будут
100000
100001
1110111

А вот переставлять их местами не рекомендуется ;)

sS ★★★★★
()
Ответ на: комментарий от sS

Вопрос этого анонимуса был явно провокационный: dmaspeed&0x21==0x21 => dmaspeed&0x20==0x20 ;)

dave ★★★★★
()
Ответ на: комментарий от anonymous

Вопрос состоит в следующем. Если мы исправим и поменяем первые две строки со следующими, то не вылезут ли новые ошибки, которые раньше себя никак не проявляли? В общем, я бы оставил как есть :)

dave ★★★★★
()
Ответ на: комментарий от sS

+1

тем более что специфика работы такая, что приходится надеяться в основном на логи. Интерактивная отладка - оч. редко.

sergej ★★★★★
()
Ответ на: комментарий от mky

>И самое главно, что плохого в этом совпадении? Если область действия локальной переменной 3 строчки, зачем придумывать ей новое имя...

А глобальные переменные - вообще глобальное зло :)

KRoN73 ★★★★★
()
Ответ на: комментарий от KRoN73

> А глобальные переменные - вообще глобальное зло :)

Локальные переменные - локальное зло. Так плавно река дискуссии впадает в бухту функциональщины;)

svu ★★★★★
()
Ответ на: комментарий от svu

Функциональное программирование - это функциональное зло :D

Рулит пребывание на лоне природы при +28 за бОртом и наличии ближайшего компа, связанного с Интернетом в 500 км :)

KRoN73 ★★★★★
()
Ответ на: комментарий от anonymous

> М.б. потому, что в большинстве случаев их просто невозможно избежать?

IMHO если компилятор выдает варнинг - это весьма плохой признак (сам считаю код потенциально приемлемым только при полном отсутствии варнингов).

no-dashi ★★★★★
()
Ответ на: комментарий от stellar

> Да, написал и немало. Еще вопросы?

На Visual Basic ?

anonymous
()
Ответ на: комментарий от no-dashi

> IMHO если компилятор выдает варнинг - это весьма плохой признак

В принципе, да, но не всегда. Например:

void *func() {
...
pthread_exit()
};


выдаст ворнинг на тему отсутствия возвращаемого значения, хотя до завершения функции выполнение не дойдет. Придется ставить какой-нибудь "return NULL;" в конце.

anonymous
()

Да, есть в его словах значительная доля истины..

MiracleMan ★★★★★
()
Ответ на: комментарий от stellar

>> return (FXbool)root_item; >Не будет работать на тех архитектурах, где sizeof(FXbool) != sizeof(type of root_item).

Вы же говорили на С++ пишите. Так при чем здесь sizeof, если можно было просто операторы поперегружать.

skwish ★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.