Re: [make-initrd] [PATCH] ueventd: Don't use a epoll timeout when it's not needed

From: Alexey Gladkov <legion@altlinux.ru>
To: make-initrd@lists.altlinux.org
Subject: Re: [make-initrd] [PATCH] ueventd: Don't use a epoll timeout when it's not needed
Date: Mon, 22 May 2023 11:19:19 +0200
Message-ID: <ZGszl0yElzKvNSEL@example.org> (raw)
In-Reply-To: <ZGsfmwSZbpVzrkOj@example.org>

On Mon, May 22, 2023 at 09:54:03AM +0200, Alexey Gladkov wrote:
> On Mon, May 22, 2023 at 07:46:07AM +0300, Leonid Krivoshein wrote:
> > Привет!
> > 
> > 
> > Глянул финальную версию в for-master. Было неплохо, а стало намного 
> > лучше. :-)
> > 
> > В ueventd.c(108): утечка path.
> 
> Да, пропустил утечку. Спасибо! 
> 
> > А в строках 124, 196, 324, 330 и 336 нет предупреждений?
> 
> Нет.
> 
> > В строке 178 получается странный путь для отслеживания
> > ("%s/queue/pause/."), поскольку параметр name="." -- так задумано или
> > тут д.б. ".all"?
> 
> Так и задумано. Может это не самое удачное решение для watch_path(). Нужна
> была возможность указывать саму директорию filter_dir. Мне не хотелось
> дополнительно копировать и подготавливать строку ради единственного вызова
> watch_path() на старте. Поэтому в этом и остальных местах в качестве name
> просто передаётся ".".
> 
> > В memory.c несколько удивляет неудобство интерфейса rd_asprintf_or_die() 
> > -- зачем тащить указатель на переменную в первом параметре и потом её 
> > возвращать? Мне кажется, тут можно обойтись локальной переменной и не 
> > тащить указатель через стек, к тому же 'char *'.
> 
> Очень хорошая мысль. Код становится более читабильным.
> 
> Правда, наткнулся на предупреждение от компилятора:
> 
> datasrc/libinitramfs/memory.c:30:7: warning: function might be candidate for attribute ‘noreturn’ [-Wsuggest-attribute=noreturn]                                                                                    
>    30 | char *rd_asprintf_or_die(const char *fmt, ...)                                                                                                                                                              
>       |       ^~~~~~~~~~~~~~~~~~
> 
> да, функция делает exit при условии ошибки vasprintf, но при этом функция
> явно делает return. Какой тут нафиг noreturn ? Я не нашёл способов
> исправить это кроме как выключить -Wmissing-noreturn.

Потому что я написал херню и так компилятор мне на это намекал. Даже
стыдно, что я сразу этого не заметил.

> > В queue-processor.c вызовы open() в 51, 55 и 59 так и остались не 
> > обёрнуты, на хорошей нагрузке это может приводить к произвольным 
> > фатальным вылетам с Interrupted system call. К слову, абсолютно все 
> > close() в коде теоретически могут быть этому подвержены.
> 
> Я передумал оборачивать open/close. open может получить на медленном
> устройстве, что маловероятно для initramfs.
> 
> -- 
> Rgrds, legion
> 
> _______________________________________________
> Make-initrd mailing list
> Make-initrd@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/make-initrd

-- 
Rgrds, legion