From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 55DFF5A004E for ; Thu, 08 Aug 2024 14:30:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723120239; 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=wQyzbDfdotBP/gboh9zeUkXjh324gXnkIWt1a8bGvAs=; b=itsDR8BYXJs50k4XPgcyMG5jtnlYcCIjQbXOY7onhwGofG+GoAJIJsAm1mqedYHasvuVtz jM03B9FX9jy0DTQGPl8ZfLxHkkqiyRTH0GwgiwEZK/ja9OIw7mTNlI3tg3hrqwNEWDmnsg vnMl48dW/40D7s9NrSO17XrUEtmlp3g= Received: from mail-pl1-f198.google.com (mail-pl1-f198.google.com [209.85.214.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-491-Ni4Y8hn9NDCg1vWZ4MYz7Q-1; Thu, 08 Aug 2024 08:30:37 -0400 X-MC-Unique: Ni4Y8hn9NDCg1vWZ4MYz7Q-1 Received: by mail-pl1-f198.google.com with SMTP id d9443c01a7336-1fc658a161bso9013295ad.0 for ; Thu, 08 Aug 2024 05:30:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723120237; x=1723725037; 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=wQyzbDfdotBP/gboh9zeUkXjh324gXnkIWt1a8bGvAs=; b=wty5r3Ln3jZ9cp5GuT+8SYgAB8kwx2Ekf7lVIc+B33U6rcAGQDu6avkrd0yfM5IGRG b1EnZVrmj5AC3MmCyJqOqtQJ1x9M0Dc1HPzGkO+4l3rrhIBEQn0wkXlj7oBo6r2M6374 u+8QHKz8qeaDB9sMaxw7ejvdLxT6KTEjCR4nqEyzT08ZJfJJUKHG5HexTzSsbrtFOmUT MRVaaCcDUyTQ+bjjtHM7v4T09vNx1iXslrVylXCyWr2VCfT8Hd75rUuOaHx8uPjl9JvE PlSVxiXzEeaIA4VX7I+NnAdvEoMdqAnpcYJ6EkNcXWo009PbQ/SjtF4UO1+7oEp6UvnM auZg== X-Gm-Message-State: AOJu0YxsXdMUm1If3hiVdhtM9i4/CBoNfWhvtk/IW4kmnD99IkOg+YFj QsZC6l3Q/XLDwczJCVC6IXPhxwF476fqkMRaXQd1Tfe4PEYcGTAZo+sMWQjcUFqZbhh0Yk2Hux5 PW3gSAlCpnzSDvhAoXLAmy+ka7fz9FhwZ/13Qit/7DbK4CFlIWA== X-Received: by 2002:a17:902:e747:b0:1fa:fbe1:284e with SMTP id d9443c01a7336-20095119a8emr19795165ad.0.1723120233343; Thu, 08 Aug 2024 05:30:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFDcz0MmZV4ZdfcnORJ8sDM6CRqLGfmWpU4pHpcb/Yut96xQlA7cNrKJYXcgYrnVdI54FY3RA== X-Received: by 2002:a17:902:e747:b0:1fa:fbe1:284e with SMTP id d9443c01a7336-20095119a8emr19794805ad.0.1723120232651; Thu, 08 Aug 2024 05:30:32 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1ff59297707sm123550415ad.254.2024.08.08.05.30.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Aug 2024 05:30:32 -0700 (PDT) Date: Thu, 8 Aug 2024 14:30:29 +0200 From: Stefano Brivio To: Paul Holzinger Subject: Re: [PATCH v3] conf: Stop parsing options at first non-option argument Message-ID: <20240808143029.2a99afc3@elisabeth> In-Reply-To: <50d8ea4a-15ec-48a0-bc31-1e2a5139677b@redhat.com> References: <20240808040251.2568850-1-sbrivio@redhat.com> <20240808121658.6574ef4c@elisabeth> <50d8ea4a-15ec-48a0-bc31-1e2a5139677b@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: QSXG6HNE4UHTIGUOXARCLG2RDPQNOP3V X-Message-ID-Hash: QSXG6HNE4UHTIGUOXARCLG2RDPQNOP3V 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 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 Thu, 8 Aug 2024 13:29:09 +0200 Paul Holzinger wrote: > On 08/08/2024 12:16, Stefano Brivio wrote: > > On Thu, 8 Aug 2024 11:37:38 +0200 > > Paul Holzinger wrote: > > > >> On 08/08/2024 06:02, Stefano Brivio wrote: > >>> Given that pasta supports specifying a command to be executed on the > >>> command line, even without the usual -- separator as long as there's > >>> no ambiguity, we shouldn't eat up options that are not meant for us. > >>> > >>> Paul reports, for instance, that with: > >>> > >>> pasta --config-net ip -6 route > >>> > >>> -6 is taken by pasta to mean --ipv6-only, and we execute 'ip route'. > >>> That's because getopt_long(), by default, shuffles the argument list > >>> to shift non-option arguments at the end. > >>> > >>> Avoid that by adding '+' at the beginning of 'optstring'. > >>> > >>> Reported-by: Paul Holzinger > >>> Signed-off-by: Stefano Brivio > >>> --- > >>> v3: Use '+' in optstring and drop first non-option tracking > >>> > >>> v2: Instead of overriding 'name' in the getopt_long() loop, to force > >>> exiting the loop, adjust the exit condition > >>> > >>> conf.c | 4 ++-- > >>> util.c | 2 +- > >>> 2 files changed, 3 insertions(+), 3 deletions(-) > >>> > >> I like this change but I like to point out that this is a breaking > >> change for any user that sets options after the main argument, i.e. pid. > > Oh, right, that actually happens to work, even if we never really > > supported that, the man page has only this form: > > > > pasta [OPTION]... PID > > > > I could go back to v2, and, on top of that, if we find a single > > non-option argument that looks like a PID (a number), we would push it > > to the end of argv and continue parsing. > > > > If we find any other number, including one that's after all the other > > options but before the presumed PID we just pushed, we'll report error. > > > > We have anyway the following problem, which we won't make any worse (it > > can't be done without an actual file with POSIX shell, Bash only): > > > > $ 1() { echo a; } > > $ pasta 1; echo > > Couldn't open user namespace /proc/1/ns/user: Permission denied > > > > $ pasta echo; 1 > > > > a > > > >> I can tell you that this will not effect podman but I don't know what > >> other users exists out there... > > As far as I know, the only other tool using pasta(1) at the moment is > > rootless-containers (Docker, Usernetes): > > > > https://github.com/rootless-containers/rootlesskit/blob/master/pkg/network/pasta/pasta.go > > > > which is also fine. Other users are developers and people who try out > > network topologies and namespaces stuff without root, but I suppose > > adding the PID at the end is pretty natural anyway. > > > > On the other hand, if we can make sure we avoid this kind of breakage > > at a small cost, why not. I'll try. > > > >> I am not sure if it is worth the risk just to improve the UX for the > >> command use case but I guess you already decided it is otherwise you > >> would have not posted this patch. > > No, not really, I wasn't actually aware of the fact that adding the PID > > before options worked. Thanks for pointing that out. > > Well not just pid, it works the same with a command: > > $ pasta ip addr --config-net > > With this patch this no longer works, as --config-net is now passed to > the ip command. I don't think using it like that makes any sense and it > is super confusing so I like the new way but whoever does use such a > syntax will get broken. Thus it is a trade off to be made. Ah, sure, I see, but that's a completely different level of unreasonableness. I don't think anybody ever dreamt of using it like that. That something I would happily "break". -- Stefano