linux-ext4 - Re: [PATCH] e2fsck: map PROMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <49C6329B-BF16-4819-98F8-AE98F985676E@dilger.ca>
Date:   Mon, 28 Feb 2022 16:58:19 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     "Darrick J. Wong" <djwong@...nel.org>
Cc:     Theodore Ts'o <tytso@....edu>,
        linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] e2fsck: map PROMPT_* values to prompt messages

On Dec 10, 2021, at 5:51 PM, Darrick J. Wong <djwong@...nel.org> wrote:
> On Thu, Dec 09, 2021 at 02:55:26PM -0700, Andreas Dilger wrote:
>> On Dec 8, 2021, at 9:42 AM, Darrick J. Wong <djwong@...nel.org> wrote:
>>> 
>>> On Wed, Dec 08, 2021 at 12:51:12AM -0700, Andreas Dilger wrote:
>>>> It isn't totally clear when searching the code for PROMPT_*
>>>> constants from problem codes where these messages come from.
>>>> Similarly, there isn't a direct mapping from the prompt string
>>>> to the constant.
>>>> 
>>>> Add comments that make this mapping more clear.
>>>> 
>>>> Signed-off-by: Andreas Dilger <adilger@...ger.ca>
>>>> ---
>>>> e2fsck/problem.c | 46 +++++++++++++++++++++++-----------------------
>>>> 1 file changed, 23 insertions(+), 23 deletions(-)
>>>> 
>>>> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
>>>> index 757b5d56..2d02468c 100644
>>>> --- a/e2fsck/problem.c
>>>> +++ b/e2fsck/problem.c
>>>> @@ -50,29 +50,29 @@
>>>> * to fix a problem.
>>>> */
>>>> static const char *prompt[] = {
>>>> -	N_("(no prompt)"),	/* 0 */
>>>> -	N_("Fix"),		/* 1 */
>>>> -	N_("Clear"),		/* 2 */
>>>> -	N_("Relocate"),		/* 3 */
>>>> -	N_("Allocate"),		/* 4 */
>>>> -	N_("Expand"),		/* 5 */
>>>> -	N_("Connect to /lost+found"), /* 6 */
>>>> -	N_("Create"),		/* 7 */
>>>> -	N_("Salvage"),		/* 8 */
>>>> -	N_("Truncate"),		/* 9 */
>>>> -	N_("Clear inode"),	/* 10 */
>>>> -	N_("Abort"),		/* 11 */
>>>> -	N_("Split"),		/* 12 */
>>>> -	N_("Continue"),		/* 13 */
>>>> -	N_("Clone multiply-claimed blocks"), /* 14 */
>>>> -	N_("Delete file"),	/* 15 */
>>>> -	N_("Suppress messages"),/* 16 */
>>>> -	N_("Unlink"),		/* 17 */
>>>> -	N_("Clear HTree index"),/* 18 */
>>>> -	N_("Recreate"),		/* 19 */
>>>> -	N_("Optimize"),		/* 20 */
>>>> -	N_("Clear flag"),	/* 21 */
>>>> -	"",			/* 22 */
>>>> +	N_("(no prompt)"),			/* PROMPT_NONE		=  0 */
>>> 
>>> Why not make it even clearer and mismerge proof:
>>> 
>>> static const char *prompt[] = {
>>> 	[0]		= N_("(no prompt")),	/* null value test */
>>> 	[PROMPT_FIX]	= N_("Fix"),		/* 1 */
>>> 	[PROMPT_CLEAR]	= N_("Clear"),		/* 2 */
>>> 	...
>>> };
>> 
>> I thought about that too, but then I thought the "[index] = foo" designated
>> initializer is GNU or at least C99-specific, and I wondered if that was
>> going to cause portability problems for some ancient system that e2fsprogs
>> is building on...  I figured adding comments is relatively safe, and these
>> values change so rarely that more complexity in the patch was not a win.
> 
> <shrug> Yeah, I thought it was safe enough to use -std=gnu90 features,
> but I guess it's really up to Ted to decide if he'd prefer a structural
> fix or not.  Evidently this syntax is /still/ being argued in the C++
> committees, which ... yay. :(

Ted, thoughts on this?

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)