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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
On Dec 8, 2021, at 9:42 AM, Darrick J. Wong <[email protected]> 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 <[email protected]>
>> ---
>> 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
On Thu, Dec 09, 2021 at 02:55:26PM -0700, Andreas Dilger wrote:
> On Dec 8, 2021, at 9:42 AM, Darrick J. Wong <[email protected]> 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 <[email protected]>
> >> ---
> >> 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
>
>
>
>
>
On Dec 10, 2021, at 5:51 PM, Darrick J. Wong <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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