From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=SBQu2REB; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 991DA5A026F for ; Tue, 21 Oct 2025 23:51:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1761083464; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Waw/QJ6tO2nlZQoL4e8D7bFs81i9O8KagjIFcr9F0HA=; b=SBQu2REBDPkhjyd47qkTZbYgmiAoLDJTQnu+JdX5R53dp1tlzBp8R9WZX8TFkV+kT6/N/1 JbyfEMkDQJ+dz9pDYigTmqcxmHOWHPUjHm7/0gUR2bGJKQjtHWkhK+gJkG28ubU6dVkF/S ZQt2Cnq6iJ4q+iAxpnk1lgs3GnG04so= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-678-GDWQH_9GMqSkxzxctZ8mWA-1; Tue, 21 Oct 2025 17:51:02 -0400 X-MC-Unique: GDWQH_9GMqSkxzxctZ8mWA-1 X-Mimecast-MFC-AGG-ID: GDWQH_9GMqSkxzxctZ8mWA_1761083462 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-40fd1b17d2bso3509888f8f.1 for ; Tue, 21 Oct 2025 14:51:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761083461; x=1761688261; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=A9E1xZPGooO37YHY5lSAE/G21rAPIvWfyoSgsI2vmPE=; b=aPGDSsYcf7sb/9CGscMcchwC8cM2PPVFi2gyCdPwdHmzH2YgtZ2luoQrgY166g9jcp 7rMQLAs1oVlole/TmCpHlHFyazEwjVcykB75OhC2QZ9683rIAF53uBp6FFCuag1pfRqr KgxAbPQyLECVFgxwKC1yK/PH4Tce/0Vbqg6c+08MgtdspvDuhct0zarNeDyqkmCFqUjQ MzNNOMiLgJfg8w7Rv6LxUy0cSjqX6EIuf+mRgXroccQZYDUZYwNfNThC0Qc3BJM90P3n b1cqeJC2L+Dz5DF24AYflGknhpM8BEfAOeaNFPJji2rrLLjGq3TlaR5jKJyfs2lM28+z JmuQ== X-Gm-Message-State: AOJu0YzFVjOypml/LvJOoZ2CWjZXlCgK19GGI2yYmgkXJKlQBBH17w6x SO2fq68b0BMN0fS9XhatuDEfengAQQFUrtaOknaUpYv2ZylxJDcWQlgTAKoiUP9Mmuw8MTwbtXU aqWbRveEn5tDQpeANY0tAdxF4OVpF75xhTrCX7v08Joi1AntBUok1TA== X-Gm-Gg: ASbGncsXQpog6q9NlB1nWkVfNvUa2v+/6XQdi5o+5FhOeOlHNK/CE/XA0z98H64WiLY YZJtkayxz5OcshBfOt6Rz7MkxAqNLSPR0Ogv1N52RH1/6KlhQFfyPNvE1aJCMik/e+fMt7DWToZ n6YTICH5vbonLsP4KaAuqQ0alImIottaM3WChahHkXOYVW/LYmo0JA7rNzLUTY0hS/Z0hs8HysC CHCHP97MRE8cKAvt/ZZQUs5POPCZVmcjd9Ajs8Lbwmn5QR/Yp6OlLXCFtivVhk621hnCVERpTPA 4C8Ui0pgVD3L2+n3TI3u589cOSk+XH2gRRUKhD/a/cXNYRRRpM03IAKAoFT5EWRvhzUygi1AOK/ IIlp3DWIm6PS9V5JnjcEhbHdhRME= X-Received: by 2002:a05:6000:4718:b0:426:f10e:6b56 with SMTP id ffacd0b85a97d-42704db1df0mr13553298f8f.27.1761083461464; Tue, 21 Oct 2025 14:51:01 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGET6dSZa2TM3T97zGVLpRZifsnHBHvmgFhHfpuKPiTyuCyQs5W8aS2oIC4q0rnT1emwsmzaQ== X-Received: by 2002:a05:6000:4718:b0:426:f10e:6b56 with SMTP id ffacd0b85a97d-42704db1df0mr13553281f8f.27.1761083460933; Tue, 21 Oct 2025 14:51:00 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-427f009a976sm22519429f8f.32.2025.10.21.14.51.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Oct 2025 14:51:00 -0700 (PDT) Date: Tue, 21 Oct 2025 23:50:59 +0200 From: Stefano Brivio To: Yumei Huang Subject: Re: [PATCH v6 2/4] util: Introduce read_file() and read_file_integer() function Message-ID: <20251021235059.0c5244e8@elisabeth> In-Reply-To: References: <20251017062838.21041-1-yuhuang@redhat.com> <20251017062838.21041-3-yuhuang@redhat.com> <20251019120712.6f232804@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: T6ltUGXrVW9lM0sY6N6otMlMbuVqmOOVv5vhtknq99Q_1761083462 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 5RHMTBFHZAYIPX63GPCFNOTQQLFCPKT4 X-Message-ID-Hash: 5RHMTBFHZAYIPX63GPCFNOTQQLFCPKT4 X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, david@gibson.dropbear.id.au X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Tue, 21 Oct 2025 17:32:58 +0800 Yumei Huang wrote: > On Sun, Oct 19, 2025 at 6:07=E2=80=AFPM Stefano Brivio wrote: > > > > On Fri, 17 Oct 2025 14:28:36 +0800 > > Yumei Huang wrote: > > =20 > > > Signed-off-by: Yumei Huang > > > --- > > > util.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++ > > > util.h | 8 ++++++ > > > 2 files changed, 92 insertions(+) > > > > > > diff --git a/util.c b/util.c > > > index c492f90..5c8c4bc 100644 > > > --- a/util.c > > > +++ b/util.c > > > @@ -579,6 +579,90 @@ int write_file(const char *path, const char *buf= ) > > > return len =3D=3D 0 ? 0 : -1; > > > } > > > > > > +/** > > > + * read_file() - Read contents of file into a buffer > > > + * @path: File to read > > > + * @buf: Buffer to store file contents > > > + * @buf_size: Size of buffer > > > + * > > > + * Return: number of bytes read on success, -1 on any error, -2 on t= runcation > > > +*/ > > > +ssize_t read_file(const char *path, char *buf, size_t buf_size) > > > +{ > > > + int fd =3D open(path, O_RDONLY | O_CLOEXEC); > > > + size_t total_read =3D 0; > > > + ssize_t rc; > > > + > > > + if (fd < 0) { > > > + warn_perror("Could not open %s", path); > > > + return -1; > > > + } > > > + > > > + while (total_read < buf_size) { > > > + rc =3D read(fd, buf + total_read, buf_size - total_read= ); > > > + > > > + if (rc < 0) { > > > + warn_perror("Couldn't read from %s", path); > > > + close(fd); > > > + return -1; > > > + } > > > + > > > + if (rc =3D=3D 0) > > > + break; > > > + > > > + total_read +=3D rc; > > > + } > > > + > > > + close(fd); > > > + > > > + if (total_read =3D=3D buf_size) { > > > + warn("File %s truncated, buffer too small", path); > > > + return -2; > > > + } > > > + > > > + buf[total_read] =3D '\0'; > > > + > > > + return total_read; > > > +} > > > + > > > +/** > > > + * read_file_integer() - Read an integer value from a file > > > + * @path: File to read > > > + * @fallback: Default value if file can't be read > > > + * > > > + * Return: Integer value, fallback on failure > > > +*/ > > > +intmax_t read_file_integer(const char *path, intmax_t fallback) > > > +{ > > > + char buf[INTMAX_STRLEN]; > > > + ssize_t bytes_read; > > > + intmax_t value; > > > + char *end; > > > + > > > + bytes_read =3D read_file(path, buf, sizeof(buf)); > > > + > > > + if (bytes_read < 0) > > > + return fallback; > > > + > > > + if (bytes_read =3D=3D 0) { > > > + debug("Empty file %s", path); > > > + return fallback; > > > + } > > > + > > > + errno =3D 0; > > > + value =3D strtoimax(buf, &end, 10); > > > + if (*end && *end !=3D '\n') { > > > + debug("Invalid format in %s", path); > > > + return fallback; > > > + } > > > + if (errno) { > > > + debug("Invalid value in %s: %s", path, buf); > > > + return fallback; > > > + } > > > + > > > + return value; > > > +} > > > + > > > #ifdef __ia64__ > > > /* Needed by do_clone() below: glibc doesn't export the prototype of= __clone2(), > > > * use the description from clone(2). > > > diff --git a/util.h b/util.h > > > index 22eaac5..3f9f296 100644 > > > --- a/util.h > > > +++ b/util.h > > > @@ -222,6 +222,8 @@ void pidfile_write(int fd, pid_t pid); > > > int __daemon(int pidfile_fd, int devnull_fd); > > > int fls(unsigned long x); > > > int write_file(const char *path, const char *buf); > > > +ssize_t read_file(const char *path, char *buf, size_t buf_size); > > > +intmax_t read_file_integer(const char *path, intmax_t fallback); > > > int write_all_buf(int fd, const void *buf, size_t len); > > > int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, = size_t skip); > > > int read_all_buf(int fd, void *buf, size_t len); > > > @@ -250,6 +252,12 @@ static inline const char *af_name(sa_family_t af= ) > > > > > > #define UINT16_STRLEN (sizeof("65535")) > > > > > > +/* Each byte expands to at most 3 decimal digits since 0xff =3D=3D 2= 55. > > > + * Plus 2 extra bytes for the sign and null terminator. > > > + * See https://stackoverflow.com/a/10536254. =20 > > > > This is not an acceptable form of attribution according to the > > CC BY-SA 3.0 terms. See: > > > > https://stackoverflow.com/help/licensing > > https://creativecommons.org/licenses/by-sa/3.0/ > > > > and checksum.h in this tree for some examples of how to combine > > different licensing terms in a single file, in a way that's > > human-readable but still machine-friendly (for license / compliance > > scanners such as REUSE). > > > > As I commented on a previous version, anyway, I don't think we need > > this at all. I guess my comment was ignored though. =20 >=20 > I guess you meant the comment of suggesting using BUFSIZ in V2? Right, but on v4, and that was just Friday for everybody involved... > David replied as quote: >=20 > "We could use BUFSIZ, but it's massive overkill for > reading a single integer: 8192 versus ~21 bytes (or ~42 bytes if > intmax_t were 128-bit)." I wanted to reply to that because sure, BUFSIZ is typically 8192 bytes on glibc and 1024 with musl, but adding 10 or 8192 to the stack pointer doesn't really make a difference. It's not like we allocate that memory anyway, and I don't think any of that memory (or unused holes on the stack we create) is prefetched. And regardless of all that... we don't use these functions on any data path, it's just during configuration. It can be (relatively) slow. > It's my fault. I should have waited to see if you have more comments > before sending the new version. Thanks for understanding. My biggest problem with this is: where do I comment now? On v4 or v6? Should I even read v5? Well, anyway, I'll directly review v6 (in a bit) to keep things sane. --=20 Stefano