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=cSEIfPIb; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id BDFB35A0274 for ; Tue, 18 Nov 2025 01:19:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1763425183; 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=R2PdzlVcvxYZImxabmz0YMezf3aXouHk5EmclVoPvS4=; b=cSEIfPIbkmGw1+/cyCtlKdYZgTZTNFCq77gROQVUxNT9ysa2TsoKT33hqkzVVwoadHDcki BWZ4K6NOgp0grGTpbCslhksyOQP+YvDzOLBcwHsD7tVLpRDzd6K3AUHQi7PfFgc+2zBXVf 4sc98a1/bw96jGYMH+Hf/m1u+8OdJVc= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-319-mZTpoB9sNMewKmPv9tJn1w-1; Mon, 17 Nov 2025 19:19:42 -0500 X-MC-Unique: mZTpoB9sNMewKmPv9tJn1w-1 X-Mimecast-MFC-AGG-ID: mZTpoB9sNMewKmPv9tJn1w_1763425181 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-42b2b642287so2795311f8f.2 for ; Mon, 17 Nov 2025 16:19:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763425181; x=1764029981; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=R2PdzlVcvxYZImxabmz0YMezf3aXouHk5EmclVoPvS4=; b=sl6bncPzrCcp6Spd5TXrIpgk1MMzf3Q7R+oASB/OF3M3kI3lf1+xzWajIJNE4PsZqJ 6hAQKRvdTvyVDAOU5Vd8lFEJHpSLNApzXgKlkHd4skXsmLy+Y6E2Cpysof+OTxoXYn3U 0Gmz3+5DKjXHArAyl4YJg3GVUfY9caAt5o8GncZNLohUzeyULqr06VjLoyJn4uhbmkWa 6L+yML3QpHtXDs6DPNlC+hTrrZ2dM8tjr3RfkXEL8FGmVUUsqSR8VsT5BBMFxWNnIud6 01XmraNo4Il7d6z9S3LigtVNbWzN5u13QbOBwNYH/+RxyDA58Hz5vACKTFGlelaWUW0M j2RQ== X-Gm-Message-State: AOJu0YwXQpZdTGJCyTLvIeZ8aCJ57Wem+QeXBIqSjLmhtlF60hXg0jO+ kKyGa6FKMIY9JTcTWzwpMjH69g6DJmxS/ADBjclqxxNzrW0O+GiS7HFnWNJq2MUvrP3zaVmAzlf maat1PtxuyTSy/UUluMaYkAn6HiYhGPOJ9h85iQOv/TJTCjjbLtcTlQ== X-Gm-Gg: ASbGncs3U57c1Fyp9N8d9YnJaZI4wuxnQ4N7KhDvZI+9bTRA3vyJuaSObU+oPp0+/pt kyYkzjB+OXyJS6L84rIuhG77lMZ+cPGZU+KqH4egVonKadqzgZbD0G0nwx/oUtjKDz6jkQlKBmq Pa+cBX46lQCPzSLir4vojuwL4eyyPdtucn2SaMPgu5OcYHvsT1nNkC1xwISeMvqPoaeC5nYj443 U3XIOQrjnExa9W34J0K6cEPQCIUt/zRdW2cjCl1uIEbYpUaeVHdXOVNyQmt8TEzjKu1QKYk1oyO 2e29djvmhVK53LNNgDcV6B3Xb24htRnJ2K3gTu8ij1T+NKE4/pYEi4LXCMzDYblbNbQvrQ/rQ+f TwrZVe5nNkVAG4PeqMDTbGjiB8rP1lsnucMmZBw== X-Received: by 2002:a05:6000:2911:b0:429:b9bc:e810 with SMTP id ffacd0b85a97d-42b59388427mr13441824f8f.45.1763425180987; Mon, 17 Nov 2025 16:19:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IHzdHxZzO1L5X438z35TCDgdAfDIiuR4GNYUSyDCR3LrJDaV6g5UgI7jAzLcHmeY4XqBjd1wg== X-Received: by 2002:a05:6000:2911:b0:429:b9bc:e810 with SMTP id ffacd0b85a97d-42b59388427mr13441808f8f.45.1763425180451; Mon, 17 Nov 2025 16:19:40 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-42b53f2084dsm29451515f8f.42.2025.11.17.16.19.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Nov 2025 16:19:39 -0800 (PST) Date: Tue, 18 Nov 2025 01:19:38 +0100 From: Stefano Brivio To: Yumei Huang Subject: Re: [PATCH v8 2/6] util: Introduce read_file() and read_file_integer() function Message-ID: <20251118011938.47d5c1e2@elisabeth> In-Reply-To: <20251110093137.87705-3-yuhuang@redhat.com> References: <20251110093137.87705-1-yuhuang@redhat.com> <20251110093137.87705-3-yuhuang@redhat.com> 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: S2cCb_x_ytpi-uaikauD-FI9EDJ3lGYF2EPr1oLerZg_1763425181 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: CSLM4I2OARSF5HJKTVGMCMICIA5NOCVO X-Message-ID-Hash: CSLM4I2OARSF5HJKTVGMCMICIA5NOCVO 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 Mon, 10 Nov 2025 17:31:33 +0800 Yumei Huang wrote: > Signed-off-by: Yumei Huang > Reviewed-by: David Gibson > --- > util.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > util.h | 2 ++ > 2 files changed, 88 insertions(+) > > diff --git a/util.c b/util.c > index 44c21a3..c4c849c 100644 > --- a/util.c > +++ b/util.c > @@ -590,6 +590,92 @@ int write_file(const char *path, const char *buf) > return len == 0 ? 0 : -1; > } > > +/** > + * read_file() - Read contents of file into a NULL-terminated buffer > + * @path: Path to file to read > + * @buf: Buffer to store file contents > + * @buf_size: Size of buffer > + * > + * Return: number of bytes read on success, -1 on error, -ENOBUFS on truncation > + */ > +ssize_t read_file(const char *path, char *buf, size_t buf_size) > +{ > + int fd = open(path, O_RDONLY | O_CLOEXEC); > + size_t total_read = 0; > + ssize_t rc; > + > + if (fd < 0) { > + warn_perror("Could not open %s", path); While testing something unrelated I realised that this looks like a serious failure, but it's perfectly normal on (even slightly) older kernels: --- $ git describe --contains ccce324dabfe # tcp: make the first N SYN RTO backoffs linear v6.5-rc1~163^2~299 $ git describe --contains 1280c26228bd # tcp: add tcp_rto_max_ms sysctl v6.15-rc1~160^2~352^2 --- On a 6.1 kernel, for example: --- $ ./pasta -d --config-net [...] 0.0258: Could not open /proc/sys/net/ipv4/tcp_syn_linear_timeouts: No such file or directory 0.0258: Could not open /proc/sys/net/ipv4/tcp_rto_max_ms: No such file or directory 0.0258: Read sysctl values syn_retries: 6, syn_linear_timeouts: 4, rto_max: 120 --- and actually the third message isn't accurate, because we didn't read those values, we are just using them. Worse, yet: --- $ ./pasta Could not open /proc/sys/net/ipv4/tcp_syn_linear_timeouts: No such file or directory Could not open /proc/sys/net/ipv4/tcp_rto_max_ms: No such file or directory --- so this would be logged *with default options* by Podman, rootlesskit / Docker, via libvirt, and by other users too, meaning we would get submerged by reports about this "error" if we release it like this (6.15 is fairly recent). So maybe we could turn this (and the messages below) to debug() / debug_perror(), and then: > + return -1; > + } > + > + while (total_read < buf_size) { > + rc = 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 == 0) > + break; > + > + total_read += rc; > + } > + > + close(fd); > + > + if (total_read == buf_size) { > + warn("File %s contents exceed buffer size %zu", path, > + buf_size); > + buf[buf_size - 1] = '\0'; > + return -ENOBUFS; > + } > + > + buf[total_read] = '\0'; > + > + return total_read; > +} > + > +/** > + * read_file_integer() - Read an integer value from a file > + * @path: Path to 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) > +{ > + ssize_t bytes_read; > + char buf[BUFSIZ]; > + intmax_t value; > + char *end; > + > + bytes_read = read_file(path, buf, sizeof(buf)); > + > + if (bytes_read < 0) > + return fallback; ...say something here, like "using %i as default value", so that it's clear that the error doesn't have further consequences. We should probably do that for all the failure cases here, so you might want to 'goto error;' here, and also: > + > + if (bytes_read == 0) { > + debug("Empty file %s", path); here, and below, and then: > + return fallback; > + } > + > + errno = 0; > + value = strtoimax(buf, &end, 10); > + if (*end && *end != '\n') { > + debug("Non-numeric content in %s", path); > + return fallback; > + } > + if (errno) { > + debug("Out of range value in %s: %s", path, buf); > + return fallback; > + } > + > + return value; error: debug("Using ...") return fallback; > +} > + > #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 a0b2ada..c1502cc 100644 > --- a/util.h > +++ b/util.h > @@ -229,6 +229,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); -- Stefano