Linux I2C development
 help / color / mirror / Atom feed
From: "Brigham Campbell" <me@brighamcampbell.com>
To: "Gero Schwäricke" <gero.schwaericke@sevenlab.de>,
	"Brigham Campbell" <me@brighamcampbell.com>,
	"Jean Delvare" <jdelvare@suse.de>,
	linux-i2c@vger.kernel.org
Cc: "Wolfram Sang" <wsa+renesas@sang-engineering.com>
Subject: Re: [PATCH v4 1/2] i2c-tools: Allow passing device file paths
Date: Sun, 19 Jul 2026 16:22:53 -0600	[thread overview]
Message-ID: <DK2WDITRVN2Z.2QAZXR6BWBFB1@brighamcampbell.com> (raw)
In-Reply-To: <DK0V64S4JWAS.3PNF8V2ZQEDPY@sevenlab.de>

Hi Gero,

On Fri Jul 17, 2026 at 7:00 AM MDT, Gero Schwäricke wrote:
> On Thu Jul 16, 2026 at 4:49 PM CEST, Brigham Campbell wrote:
>> 	if (errno != ENOENT) {
>> 		fprintf(stderr, "Error: Could not open file "
>> 			"`%s': %s\n", i2cbus_arg, strerror(errno));
>> 		if (errno == EACCES)
>> 			fprintf(stderr, "Run as root?\n");
>> 		return file;
>> 	}
>>
>> 	fprintf(stderr, "Error: `%s' is not a bus number, name, or device file "
>> 		"path!\n", i2cbus_arg);
>
> I'm unsure about this: Yes, this reflects the behavior of
> `open_i2c_dev_by_nr()`, but we only call that after we have validated
> that the input is indeed an adapter number.

The input is not necessarily an adapter number. We call
open_i2c_dev_by_nr after parsing the argument as an unsigned long, but
that's no guarantee that the input is an adapter number.  For example,
if a system has /dev/i2c-0 and /dev/i2c-1, `i2cdetect -F 100` will
dutifully try to open /dev/i2c-100 and eventually fail without trying to
interpret the number as a bus name. That's ok because it's just a
heuristic and we don't expect the collision of file paths, bus names,
and unsigned integers.

>                                             With the path we don't do
> that, we just try to open the input as a path. We know it's not a valid
> integer, and not a valid adapter name, but it may still not be a path,
> maybe it's a mistyped adapter name.
>
> To that I'm not sure we can conclude that `errno != ENOENT` means it is
> indeed a path.

This is a good point. `errno != ENOENT` doesn't necessarily mean that
the parameter is a file and that i2c-tools should be able to open it.

> Looking at libgpiod (`gpiod_chip_open_lookup()`), they solved that by
> assuming that all paths must start with `/dev/`. Unsure if we want to go
> that route as well. It would definitely simplify things and I think for
> the sake of progress that whould be fine. Thoughts?

The approach you suggest assumes that when the user passes in a device
file path, the path is _not_ a relative path, devtmpfs is mounted at
`/dev/`, and if the path is a symlink, it's also in `/dev/`. Maybe these
assumptions are ok (after all, libgpiod makes them) but they seem
unnecessary.

What if we only indicate that the argument couldn't be opened as a file
specifically when open() returns EACCES? Otherwise, we'll print a
generic message, indicating that the argument couldn't be parsed as a
bus number, name, or device file path. Even when i2c-tools should have
been able to open the argument as a path, but couldn't because of some
unrelated error, the binary will print a less-specific error message
which is still applicable and correct:

	file = open(i2cbus_arg, O_RDWR);

	if (file >= 0 || quiet) return file;

	if (errno == EACCES) {
		fprintf(stderr, "Error: Could not open file `%s': %s\n"
			"Run as root?\n", i2cbus_arg, strerror(errno));
		return file;
	}

	fprintf(stderr, "Error: Couldn't interpret `%s' as a bus number, "
		"name, or device file path!\n", i2cbus_arg);

	return file;

Sorry to double down on the bikeshedding (again, I do really appreciate
the feedback)... I'm interested to hear what Wolfram has to say. I may
send out v5 (which includes the above code) along with an invitation to
either make editorial changes to the patch in case the maintainer has
something different in mind or request that I send out a v6. I have v5
queued up which includes the above snippet.

-- 
Brigham Campbell
https://brighamcampbell.com


  reply	other threads:[~2026-07-19 22:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06  4:27 [PATCH v4 0/2] i2c-tools: Make tools accept bus path Brigham Campbell
2026-07-06  4:27 ` [PATCH v4 1/2] i2c-tools: Allow passing device file paths Brigham Campbell
2026-07-09 12:18   ` Wolfram Sang
2026-07-09 12:40   ` Gero Schwäricke
2026-07-11 19:24     ` Wolfram Sang
2026-07-13  9:23       ` Gero Schwäricke
2026-07-13 15:06         ` Wolfram Sang
2026-07-14  9:30           ` Gero Schwäricke
2026-07-14 10:13             ` Wolfram Sang
2026-07-16 14:49     ` Brigham Campbell
2026-07-17 13:00       ` Gero Schwäricke
2026-07-19 22:22         ` Brigham Campbell [this message]
2026-07-13  9:37   ` Gero Schwäricke
2026-07-13 15:38     ` Brigham Campbell
2026-07-14  9:42       ` Gero Schwäricke
2026-07-16 14:17         ` Brigham Campbell
2026-07-06  4:27 ` [PATCH v4 2/2] i2c-tools: Document device paths as I2CBUS arg Brigham Campbell
2026-07-11 19:43   ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DK2WDITRVN2Z.2QAZXR6BWBFB1@brighamcampbell.com \
    --to=me@brighamcampbell.com \
    --cc=gero.schwaericke@sevenlab.de \
    --cc=jdelvare@suse.de \
    --cc=linux-i2c@vger.kernel.org \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox