All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] e2fsck: map PROMPT_* values to prompt messages
@ 2021-12-08  7:51 Andreas Dilger
  2021-12-08 16:42 ` Darrick J. Wong
  2022-05-03  3:31 ` Theodore Ts'o
  0 siblings, 2 replies; 6+ messages in thread
From: Andreas Dilger @ 2021-12-08  7:51 UTC (permalink / raw)
  To: tytso; +Cc: linux-ext4, Andreas Dilger

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@dilger.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 */
+	N_("Fix"),				/* PROMPT_FIX		=  1 */
+	N_("Clear"),				/* PROMPT_CLEAR		=  2 */
+	N_("Relocate"),				/* PROMPT_RELOCATE	=  3 */
+	N_("Allocate"),				/* PROMPT_CREATE	=  4 */
+	N_("Expand"),				/* PROMPT_EXPAND	=  5 */
+	N_("Connect to /lost+found"),		/* PROMPT_CONNECT	=  6 */
+	N_("Create"),				/* PROMPT_CREATE	=  7 */
+	N_("Salvage"),				/* PROMPT_SALVAGE	=  8 */
+	N_("Truncate"),				/* PROMPT_TRUNCATE	=  9 */
+	N_("Clear inode"),			/* PROMPT_CLEAR_INODE	= 10 */
+	N_("Abort"),				/* PROMPT_ABORT		= 11 */
+	N_("Split"),				/* PROMPT_SPLIT		= 12 */
+	N_("Continue"),				/* PROMPT_CONTINUE	= 13 */
+	N_("Clone multiply-claimed blocks"),	/* PROMPT_CLONE		= 14 */
+	N_("Delete file"),			/* PROMPT_DELETE	= 15 */
+	N_("Suppress messages"),		/* PROMPT_SUPPRESS	= 16 */
+	N_("Unlink"),				/* PROMPT_UNLINK	= 17 */
+	N_("Clear HTree index"),		/* PROMPT_CLEAR_HTREE	= 18 */
+	N_("Recreate"),				/* PROMPT_RECREATE	= 19 */
+	N_("Optimize"),				/* PROMPT_OPTIMIZE	= 20 */
+	N_("Clear flag"),			/* PROMPT_CLEAR_FLAG	= 21 */
+	"",					/* PROMPT_NULL		= 22 */
 };
 
 /*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: map PROMPT_* values to prompt messages
  2021-12-08  7:51 [PATCH] e2fsck: map PROMPT_* values to prompt messages Andreas Dilger
@ 2021-12-08 16:42 ` Darrick J. Wong
  2021-12-09 21:55   ` Andreas Dilger
  2022-05-03  3:31 ` Theodore Ts'o
  1 sibling, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2021-12-08 16:42 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: tytso, linux-ext4

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@dilger.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 */
	...
};

--D

> +	N_("Fix"),				/* PROMPT_FIX		=  1 */
> +	N_("Clear"),				/* PROMPT_CLEAR		=  2 */
> +	N_("Relocate"),				/* PROMPT_RELOCATE	=  3 */
> +	N_("Allocate"),				/* PROMPT_CREATE	=  4 */
> +	N_("Expand"),				/* PROMPT_EXPAND	=  5 */
> +	N_("Connect to /lost+found"),		/* PROMPT_CONNECT	=  6 */
> +	N_("Create"),				/* PROMPT_CREATE	=  7 */
> +	N_("Salvage"),				/* PROMPT_SALVAGE	=  8 */
> +	N_("Truncate"),				/* PROMPT_TRUNCATE	=  9 */
> +	N_("Clear inode"),			/* PROMPT_CLEAR_INODE	= 10 */
> +	N_("Abort"),				/* PROMPT_ABORT		= 11 */
> +	N_("Split"),				/* PROMPT_SPLIT		= 12 */
> +	N_("Continue"),				/* PROMPT_CONTINUE	= 13 */
> +	N_("Clone multiply-claimed blocks"),	/* PROMPT_CLONE		= 14 */
> +	N_("Delete file"),			/* PROMPT_DELETE	= 15 */
> +	N_("Suppress messages"),		/* PROMPT_SUPPRESS	= 16 */
> +	N_("Unlink"),				/* PROMPT_UNLINK	= 17 */
> +	N_("Clear HTree index"),		/* PROMPT_CLEAR_HTREE	= 18 */
> +	N_("Recreate"),				/* PROMPT_RECREATE	= 19 */
> +	N_("Optimize"),				/* PROMPT_OPTIMIZE	= 20 */
> +	N_("Clear flag"),			/* PROMPT_CLEAR_FLAG	= 21 */
> +	"",					/* PROMPT_NULL		= 22 */
>  };
>  
>  /*
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: map PROMPT_* values to prompt messages
  2021-12-08 16:42 ` Darrick J. Wong
@ 2021-12-09 21:55   ` Andreas Dilger
  2021-12-11  0:51     ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Dilger @ 2021-12-09 21:55 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Theodore Ts'o, linux-ext4

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

On Dec 8, 2021, at 9:42 AM, Darrick J. Wong <djwong@kernel.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@dilger.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.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: map PROMPT_* values to prompt messages
  2021-12-09 21:55   ` Andreas Dilger
@ 2021-12-11  0:51     ` Darrick J. Wong
  2022-02-28 23:58       ` Andreas Dilger
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2021-12-11  0:51 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: Theodore Ts'o, linux-ext4

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@kernel.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@dilger.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. :(

--D

> Cheers, Andreas
> 
> 
> 
> 
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: map PROMPT_* values to prompt messages
  2021-12-11  0:51     ` Darrick J. Wong
@ 2022-02-28 23:58       ` Andreas Dilger
  0 siblings, 0 replies; 6+ messages in thread
From: Andreas Dilger @ 2022-02-28 23:58 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Theodore Ts'o, linux-ext4

[-- Attachment #1: Type: text/plain, Size: 2855 bytes --]

On Dec 10, 2021, at 5:51 PM, Darrick J. Wong <djwong@kernel.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@kernel.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@dilger.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






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: map PROMPT_* values to prompt messages
  2021-12-08  7:51 [PATCH] e2fsck: map PROMPT_* values to prompt messages Andreas Dilger
  2021-12-08 16:42 ` Darrick J. Wong
@ 2022-05-03  3:31 ` Theodore Ts'o
  1 sibling, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2022-05-03  3:31 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: Theodore Ts'o, linux-ext4

On Wed, 8 Dec 2021 00:51:12 -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.
> 
> [...]

Applied, thanks!

[1/1] e2fsck: map PROMPT_* values to prompt messages
      commit: c1bd25333e8986b081b82a8eb8c9173a0c7480af

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-03  3:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08  7:51 [PATCH] e2fsck: map PROMPT_* values to prompt messages Andreas Dilger
2021-12-08 16:42 ` Darrick J. Wong
2021-12-09 21:55   ` Andreas Dilger
2021-12-11  0:51     ` Darrick J. Wong
2022-02-28 23:58       ` Andreas Dilger
2022-05-03  3:31 ` Theodore Ts'o

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.