Как избежать разыменования нулевого указателя, на примере одного исправления в ядре Linux

[Revertis]

Как мало текста, и как много ошибок. И нифига непонятен смысл из вашего описания своего подхода.

[izyk]

А что непонятно?
1. Можно пытаться избегать доступа по указателю со значением NULL.
2. По возможности избегать указателей со значением NULL. Например, заранее выделять память или как в статье, освобождать ее не полностью.
Конечно, не всегда так получится, но если есть такая возможность, второй подход, по моему мнению, лучше.

[SerJook]

Где продолжение?

[izyk]

git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/tty/tty_buffer.c?id=7391ee16950e772076d321792d9fbf030f921345
Указатель, на начало списка буферов, сразу инициализируется не нулевым значением и больше нигде не проверяется на значение NULL.

[DaemonI]

Позвольте спросить, а как второй вариант решает проблему? По моему никак, просто увеличилось кол-во инструкций, что может устранить race condition в данном конкретном случае на конкретной платформе.

Я когда-то собаку сьел на синхронизации данных находящихся в списках и понял, что:
— Нужно либо лочить доступ к списку на все время обращения к нему;
— Если же объем операций над данными в списке велик, то проще под локом скопировать нужный кусок во временный список, а общий ресурс разлочить. Это позволило значительно увеличить производительность в многоядерных системах, но работает это лишь в том случае, если ваш код выполняет какие-то эксклюзивные операции над данными, которые не повлияют на ход операций в другом потоке.

[izyk]

Тут проблема была в следующем:
Это код, изначально, так называемого flip буфера, пишем в один, читаем из другого, причем одновременно, потом меняем их местами, lock ставится только на момент смены буфера, на очень короткое время. Затем, количество буферов увеличилось, код изменился, но принцип остался.

что может устранить race condition

А нет больше никакого race condition. Мы просто не удаляем последний буфер, в который может идти запись, только стираем накопленные там данные.

[SaturnTeam]

небольшой code style вопрос:

struct tty_buffer *thead;
 
if (tty->buf.head == NULL)
	return;

Создается возможно неиспользуемая переменная, разве это не плохо?

[izyk]

Если вы о том, почему переменная не объявлена после if? Так принято, все переменные должны быть объявлены до кода.

[boolivar]

Если я правильно понял, список теперь просто сокращается до 1 блока, вместо того чтобы удалять его полностью и устанавливать указатель в NULL.
Тогда непонятно, зачем проверка на NULL:

if (tty->buf.head == NULL)
	return;

[izyk]

В начале, список пуст и нужно проверить, позже это исправлено. Ссылка в комментарии выше.

[SamSol]

Изначально код удалял все элементы списка по порядку с первого по последний (включительно).
Ваш код стал удалять все элементы списка по порядку с первого до последнего (но последний не удаляется).

Почему бы прямо так и не написать в коде?

while(tty->buf.head != tty->buf.tail) {
    thead = tty->buf.head;
    tty->buf.head = thead->next;
    tty_buffer_free(tty, thead);
}

[SamSol]

Еще одно отличие. В оригинальном коде во время вызова tty_buffer_free в tty->buf.head лежит ссылка на следующий элемент. А в вашем коде там лежит ссылка на удаляемый элемент.
ИМХО — слишком вольные изменения.

[izyk]

Согласен, так было бы яснее, но лишняя проверка не повредит.
WARN_ON(buf->head != buf->tail);

[SamSol]

Ага, представьте как будет выглядеть эта проверка после while:

while(buf->head != buf->tail) {
...
}
WARN_ON(buf->head != buf->tail);

Хочу сказать, что проверка стала не лишней, а бессмысленной.

[izyk]

А если в коде ошибка или сбой произошел и tty->buf.tail == NULL, вернемся к тому с чего начали.
Или после сбоя, ошибки всегда tty->buf.tail != tty->buf.head — зависнем под lock с запрещенными перерываниями.
Хотя я уже написал, что так было бы яснее и возможно лучше.

[andy_shev]

Там этого WARN_ON уже давно нет. Смотрите 47aa658.

[andy_shev]

Ой бяда, английский в вашем commit message хромает. Ну ничего, сам такой :)
Теперь по сути.

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

Дело в том, что изменения вносятся в основном «тусовкой». Если вы — человек со стороны, а я так вижу по вашим 3 изменениям, то порог входа будет не быстр, а ошибки устранять надо. Ну, и ваш выбор подсистемы tty — одной из самых старейших, конечно, вносит свои проблемы. Кстати, помните, что именно эта подсистема держала kernel global lock в ядре и только относительно недавно была исправлена. Т.е. я хочу сказать, что код там очень древний и очень запутанный.