From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on sa.local.altlinux.org X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM autolearn=ham autolearn_force=no version=3.4.1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684125512; x=1686717512; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=pcObrfVnIMUde47JQg0DoIM5hhy7SeVaSxRuX3qPlAo=; b=Q2Gsgopme912Vg9UHvww1hBTauxXGQXiFWg0fgPYkT0o16yZJe2xUxeoIJMDY/y4Xy TcrcyynRnUD5ot4LiYdGF7TZEatZ78h1X6O4Y/3rpovv8aXM3V9G2KVM5ZniCUCG17vx aGpUqFAZMk+02XNATOUfqso6YwjKztSznXvKc/FyyKd+yRZalIUCx8Lnzm4P0mvEzWZy +ikPn/ymQ0dDX/YyduuaXoihSBGZUr8I0ICa3/ErWh5iTls8kZ+LRBMLslROu/tj2HNR QWZinutB33vg5bmahEFd4qXklz4tSK6C6jkaL5Rd2sYPX1MAdf/d8CIhqQzMySPYOZU2 0yBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684125512; x=1686717512; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=pcObrfVnIMUde47JQg0DoIM5hhy7SeVaSxRuX3qPlAo=; b=A9vfMlxEnqk2Hvt9AGti5U5+1+4tFD1sduRHobXyqkl2ySnnr9lyJWuHyYF4xvBw8Q RtRSStPdO1ABA+V+5BOV5M2GgnfUbcbYvGpwUeqXcSZ6MhbaO1bR1Fw/XSQ+OKbYtOKb 9bU+eUpIgbbRZTW8iMDCbi29MiMTSCqzx2UuX/4tR+VeEYVyfGG/ERX+9RISHSJuPLyb HT4aLCzRO5fgo99K+oDWnDcbgiXUioVKzxlFErWdeu6KRfPbs3oWMNvX20G1P1n77NBC QmmRl3R3/V0U5D8V6/hXOhFAyKeZUfMsyJPQr4MQt3B8HWOHQ+S4Zd1o0CYV9aml2WWE Yy5Q== X-Gm-Message-State: AC+VfDwXb3ZqnxvR0zKf9HyiprLwapUB0JhQNet+3wy+ec+Jpv27f206 0tZZ7MauDKJ9+fu+arnL2tNZtFR22Dw= X-Google-Smtp-Source: ACHHUZ4E5VJAIsrG5ZNDInXObN1lHrXNVbIJtaatI4OMbTKsNvqBjA3jOy7fnCSX2dXbhEQAPgIcQQ== X-Received: by 2002:a05:6808:192a:b0:394:672b:229 with SMTP id bf42-20020a056808192a00b00394672b0229mr7394523oib.3.1684125512049; Sun, 14 May 2023 21:38:32 -0700 (PDT) Message-ID: Date: Mon, 15 May 2023 07:38:30 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.10.0 Content-Language: ru, en-US To: make-initrd@lists.altlinux.org References: <6b444ed922286eb3df8f5322b1bddf9c55753eb8.1683200226.git.gladkov.alexey@gmail.com> From: Leonid Krivoshein In-Reply-To: <6b444ed922286eb3df8f5322b1bddf9c55753eb8.1683200226.git.gladkov.alexey@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [make-initrd] [PATCH 1/3] Reimplement ueventd X-BeenThere: make-initrd@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: make-initrd@lists.altlinux.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 May 2023 04:38:35 -0000 Archived-At: List-Archive: On 5/4/23 16:42, Alexey Gladkov wrote: > [...] > diff --git a/datasrc/ueventd/queue-processor.c b/datasrc/ueventd/queue-processor.c > new file mode 100644 > index 00000000..ab5e03e4 > --- /dev/null > +++ b/datasrc/ueventd/queue-processor.c > @@ -0,0 +1,116 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > + > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "ueventd.h" > + > +static void event_handler(struct watch *queue, char *path) __attribute__((nonnull(1, 2))); > +static void move_files(char *srcdir, char *dstdir) __attribute__((nonnull(1, 2))); > +static void signal_unhandled_events(struct watch *queue) __attribute__((nonnull(1))); > + > +void event_handler(struct watch *queue, char *path) Здесь оба указателя могут быть константными, т.к. данные не меняются и дальше не передаются. > +{ > + char *argv[] = { handler_file, path, NULL }; > + pid_t pid = vfork(); > + > + if (pid < 0) { > + err("fork: %m"); > + > + } else if (!pid) { > + execve(argv[0], argv, environ); Компилятор не ругается здесь только потому, что подключен через "ueventd.h", а ругаться у него целых два повода (execve и environ). Это я к тому, что в твоём заголовочном файле он подключен для logging.c, а не для queue-processor.c. Согласно man 7 environ можно не объявлять эту переменную в коде, если подключается с _GNU_SOURCE, т.е. как раз твой случай. > + fatal("exec: %s: %m", argv[0]); > + } else { > + int status = 0; > + > + if (waitpid_retry(pid, &status, 0) != pid || !WIFEXITED(status)) > + info("%s: session=%lu: %s failed", > + queue->q_name, session, handler_file); Насчёт "session=%lu..." только лишь глядя в этот код понял, что и сам в не публичном коде опрометчиво использую uint64_t с таким форматированием, хотя везде по коду должно быть "session=" PRIu64 "...". Нужно подключить заголовочный файл либо использовать какое-то приведение типов. Пренебречь этим, как сейчас, можно только полагаясь на конкретные платформы. > + else > + info("%s: session=%lu: %s finished with return code %d", > + queue->q_name, session, handler_file, WEXITSTATUS(status)); > + } > +} > + > +void move_files(char *srcdir, char *dstdir) Здесь оба указателя тоже могут быть константными. > +{ > + struct dirent *ent; > + int srcfd, dstfd; > + > + if ((srcfd = open(srcdir, O_RDONLY | O_CLOEXEC)) < 0) > + fatal("open: %s: %m", srcdir); > + > + errno = 0; > + if ((dstfd = open(dstdir, O_RDONLY | O_CLOEXEC)) < 0) { > + if (errno == ENOENT) { > + if (mkdir(dstdir, 0755) < 0) > + fatal("mkdir: %s: %m", dstdir); > + dstfd = open(dstdir, O_RDONLY | O_CLOEXEC); > + } > + if (dstfd < 0) > + fatal("open: %s: %m", dstdir); > + } > + > + DIR *d = fdopendir(srcfd); > + if (!d) > + fatal("fdopendir: %m"); > + > + while ((ent = xreaddir(d, srcdir)) != NULL) { > + if (ent->d_name[0] != '.' && ent->d_type == DT_REG && Поле d_type заполняется не на всех системах (не на всех файловых системах), доверять можно только d_name и d_ino. Если всё это происходит на ramfs и есть уверенность, что в ней это поддерживается, то и нет проблемы... пока в этот ramfs не смонтировать что-то ещё и не начать использовать этот же код для другой файловой системы. > + renameat(srcfd, ent->d_name, dstfd, ent->d_name) < 0) > + fatal("rename `%s/%s' -> `%s/%s': %m", srcdir, ent->d_name, dstdir, ent->d_name); > + } > + > + closedir(d); > + close(dstfd); > +} > + > +void signal_unhandled_events(struct watch *queue) И здесь указатель может быть константным. > +{ > + ssize_t len; > + > + len = write_loop(queue->q_parentfd, > + (char *) &queue->q_watchfd, sizeof(queue->q_watchfd)); > + if (len < 0) > + err("write(pipe): %m"); > + > + info("%s: session=%lu: retry with the events remaining in the queue", queue->q_name, session); > +} Не углубляясь в код ueventd.c (только по названию и содержимому функции) вообще нельзя понять, что она делает и причём тут PIPE. А всё потому, что функция утилитарная и в "ueventd.h" поля структуры watch не имеют комментариев. Поэтому спрошу. Она сообщает о необработанных эвентах? Похоже, что через PIPE родительскому процессу передаётся целое число (q_watchfd). Но почему для передачи через PIPE тут используется write_loop()? Ведь в/в через PIPE (даже неблокирующий) всегда транзакционен, если не превышает PIPE_BUF. > + > +void process_events(struct watch *queue) Похоже, тут тоже указатель может быть константным. > +{ > + info("%s: session=%lu: processing events", queue->q_name, session); > + > + char *numenv = NULL; > + > + xasprintf(&numenv, "SESSION=%lu", session); > + putenv(numenv); Здесь возможна утечка: пропущен free(numenv), а когда numenv будет освобождена? > + > + char *srcdir, *dstdir; Для numenv есть инициализация, а тут нет, хотя использование одинаковое. Нет предупреждений? > + > + xasprintf(&srcdir, "%s/%s", filter_dir, queue->q_name); > + xasprintf(&dstdir, "%s/%s", uevent_dir, queue->q_name); > + > + move_files(srcdir, dstdir); > + > + if (!empty_directory(dstdir)) { > + event_handler(queue, dstdir); > + > + if (!empty_directory(dstdir)) > + signal_unhandled_events(queue); > + } > + > + free(srcdir); > + free(dstdir); > + > + exit(0); > +} > [...] -- WBR, Leonid Krivoshein.