From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) by sourceware.org (Postfix) with ESMTPS id B0D093817FF2 for ; Tue, 28 Jun 2022 14:19:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B0D093817FF2 Received: by mail-qv1-xf31.google.com with SMTP id c1so20107806qvi.11 for ; Tue, 28 Jun 2022 07:19:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=1PU9QgWjv0DC3N1nRueKGc3jsk9ippPYco96pbH3dhk=; b=1wTTwBHQORz9gNXsy6qUOGvEPSQvEuaaZn1amiKfsmV24Y0ZdR8wVKoEY/BuZ/7HxO f888ChbsAKzUFIZaN14ehH92hMEwT9CcI/g7tPYKxhzueLBKEiC0EG62TEb7SrkI2e71 7uqEWyOi8B2fqWvH0rfrSWlWQh8qX2sQad8NohguYH1UNcJJkg98jwhzGpOnsvKtNYHt XH4gbLCZstEI5Zn/e3SwAZmhlz0RA2ZKwbj9acZTTlfNaNFeDOcxZP+pD9Nox/0E+qF7 qtC+OZUpNaBgPsrVAGct/JUPWGf52RkYunY0BraoaVMGoHEKqtWPhPmNhsEk0E8+dOZd kWpw== X-Gm-Message-State: AJIora8ZE5RZuzXt+hUNh/lNHZj9CDCVIHlfFZv0Fuv3BT5xYhu/NLfp XZtrt6FS1NdJNs4fXY/6vKPEFZFgAaPyFcVvFXrBxA== X-Google-Smtp-Source: AGRyM1sDhG/EF7aAaPX1R0UAQmj6AfWjOZUYPvDYG9qmY9YXBq/5UtWhYvbbk/aYlYrvc0WfuONrbpslK8dVroJ2zIA= X-Received: by 2002:a05:622a:13d1:b0:317:c964:6b1d with SMTP id p17-20020a05622a13d100b00317c9646b1dmr13302059qtk.538.1656425943128; Tue, 28 Jun 2022 07:19:03 -0700 (PDT) MIME-Version: 1.0 References: <8f8b421d-7cc5-1b4f-7e55-5f4b41716b4c@suse.com> In-Reply-To: <8f8b421d-7cc5-1b4f-7e55-5f4b41716b4c@suse.com> From: =?UTF-8?Q?Cl=C3=A9ment_Chigot?= Date: Tue, 28 Jun 2022 16:18:53 +0200 Message-ID: Subject: Re: [PATCH] windres: add quotes around preprocessor cmd if needed To: Jan Beulich Cc: binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jun 2022 14:19:05 -0000 On Tue, Jun 28, 2022 at 3:52 PM Jan Beulich wrote: > > On 28.06.2022 14:26, Cl=C3=A9ment Chigot wrote: > > On Tue, Jun 28, 2022 at 11:37 AM Cl=C3=A9ment Chigot wrote: > >> > >> On Tue, Jun 28, 2022 at 11:00 AM Jan Beulich wrote= : > >>> > >>> On 28.06.2022 09:35, Cl=C3=A9ment Chigot via Binutils wrote: > >>> Shouldn't you also (optionally) quote the pre-processor string > >>> if that came into read_rc_file() as non-NULL? Everything else looks > >>> okay to me. > >> > >> Yeah. I didn't run into this issue but looking at the code it should h= appen > >> too. Thanks for pointing it out ! > > > > Actually the space issue is already handled by windres.c: > > https://sourceware.org/git/?p=3Dbinutils-gdb.git;a=3Dblob;f=3Dbinutils/= windres.c;h=3D4df821ee8451bda70490d6626707c2bdd66eeb5f;hb=3DHEAD#l887 > > It's not as complete as it should be. But apart of "%" I'm not sure > > any other characters caught by "filename_need_quotes" can happen > > in a folder name under Windows. > > Oh, I see. I'm not sure I see a strong need for dealing with %, so > I'm wondering: Are you intending to make an updated patch, or do > you want to commit the one you've got (which I'm okay with given > your observation)? Honestly, if this patch fits you, I would merge it as is. This issue is alr= eady a corner case, but as it's hidden inside windres code and because of the existence of the "Program Files" folder can be problematic. The other has to be triggered by someone explicitly passing "--preprocessor= " with an unusual path. So honestly I don't think it would ever happen. > > Making "filename_need_quotes" more global might be a good idea > > anyway. > > Right, but that could be a separate change then. Agreed.