From: Mykola Kvach <mykola_kvach@epam.com>

Hi all,

This small series fixes two issues in parse_color_config().

The first patch makes parse failures leave the caller-visible color count
at zero.  This prevents a rejected command-line value from leaving a
partially parsed configuration behind for later init paths to consume.

The second patch rejects empty color tokens.  Previously, delimiters in
places where a color value was expected could be interpreted as color 0,
because simple_strtoul() returns zero without advancing the input pointer.
The patch checks that each parsed color value consumed input.  It also
adds the missing newline to the DT color parsing error message.

Mykola Kvach (2):
  xen/common: llc-coloring: clear color count on parse failure
  xen/common: llc-coloring: reject empty color tokens

 xen/common/llc-coloring.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

-- 
2.43.0

On 16.05.2026 17:03, Mykola Kvach wrote:
> This small series fixes two issues in parse_color_config().
> 
> The first patch makes parse failures leave the caller-visible color count
> at zero.  This prevents a rejected command-line value from leaving a
> partially parsed configuration behind for later init paths to consume.
> 
> The second patch rejects empty color tokens.  Previously, delimiters in
> places where a color value was expected could be interpreted as color 0,
> because simple_strtoul() returns zero without advancing the input pointer.
> The patch checks that each parsed color value consumed input.  It also
> adds the missing newline to the DT color parsing error message.
> 
> Mykola Kvach (2):
>   xen/common: llc-coloring: clear color count on parse failure
>   xen/common: llc-coloring: reject empty color tokens

For both of these, a question which isn't even considered in the reasoning
is whether the present behavior may be intentional. Especially for the 2nd
ISTR Stefano(?) not so long ago indicating that the behavior is indeed
intended to be this way. That may have been somewhere on Matrix rather than
on the list, though.

In any event, you didn't Cc the authors of the patch, without which it
seems unlikely that they might even notice the submission.

Jan

Hi Jan,

Thank you for the feedback.

On Mon, May 18, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 16.05.2026 17:03, Mykola Kvach wrote:
> > This small series fixes two issues in parse_color_config().
> >
> > The first patch makes parse failures leave the caller-visible color count
> > at zero.  This prevents a rejected command-line value from leaving a
> > partially parsed configuration behind for later init paths to consume.
> >
> > The second patch rejects empty color tokens.  Previously, delimiters in
> > places where a color value was expected could be interpreted as color 0,
> > because simple_strtoul() returns zero without advancing the input pointer.
> > The patch checks that each parsed color value consumed input.  It also
> > adds the missing newline to the DT color parsing error message.
> >
> > Mykola Kvach (2):
> >   xen/common: llc-coloring: clear color count on parse failure
> >   xen/common: llc-coloring: reject empty color tokens
>
> For both of these, a question which isn't even considered in the reasoning
> is whether the present behavior may be intentional. Especially for the 2nd
> ISTR Stefano(?) not so long ago indicating that the behavior is indeed
> intended to be this way. That may have been somewhere on Matrix rather than
> on the list, though.

Thank you for pointing this out.

For the first patch, my reasoning was that returning an error while
leaving a partially parsed caller-visible color count behind looks
surprising. If the value is rejected, I think later init paths should
not be able to consume the partially parsed state.

For the second patch, my reasoning was that the current behavior looks
accidental rather than an intentional extension of the syntax.

The parser comment says:

COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
RANGE ::= COLOR-COLOR

The user guide also describes this as a comma-separated list of colors
or ranges, with ranges written as hyphen-separated inclusive intervals.
I don't see an empty-token production there.

If empty tokens are intended to mean color 0, then I agree the second
patch should be dropped or reworked, and the syntax should be documented
explicitly instead. From the current in-code and user-facing docs,
rejecting them looked more consistent to me.

>
> In any event, you didn't Cc the authors of the patch, without which it
> seems unlikely that they might even notice the submission.

On Cc: I followed the submission workflow from the Xen Project
documentation, including the add_maintainers.pl.
The documentation says that, when following those steps, the scripts
will add the correct people automatically.

Apparently that was not sufficient in this case. I will add the
original authors explicitly if there is a next posting.

Best regards,
Mykola

On 18.05.2026 08:42, Mykola Kvach wrote:
> On Mon, May 18, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 16.05.2026 17:03, Mykola Kvach wrote:
>>> This small series fixes two issues in parse_color_config().
>>>
>>> The first patch makes parse failures leave the caller-visible color count
>>> at zero.  This prevents a rejected command-line value from leaving a
>>> partially parsed configuration behind for later init paths to consume.
>>>
>>> The second patch rejects empty color tokens.  Previously, delimiters in
>>> places where a color value was expected could be interpreted as color 0,
>>> because simple_strtoul() returns zero without advancing the input pointer.
>>> The patch checks that each parsed color value consumed input.  It also
>>> adds the missing newline to the DT color parsing error message.
>>>
>>> Mykola Kvach (2):
>>>   xen/common: llc-coloring: clear color count on parse failure
>>>   xen/common: llc-coloring: reject empty color tokens
>>
>> For both of these, a question which isn't even considered in the reasoning
>> is whether the present behavior may be intentional. Especially for the 2nd
>> ISTR Stefano(?) not so long ago indicating that the behavior is indeed
>> intended to be this way. That may have been somewhere on Matrix rather than
>> on the list, though.
> 
> Thank you for pointing this out.
> 
> For the first patch, my reasoning was that returning an error while
> leaving a partially parsed caller-visible color count behind looks
> surprising. If the value is rejected, I think later init paths should
> not be able to consume the partially parsed state.
> 
> For the second patch, my reasoning was that the current behavior looks
> accidental rather than an intentional extension of the syntax.

That was my impression as well, hence why I had raised the question back
then.

> The parser comment says:
> 
> COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
> RANGE ::= COLOR-COLOR
> 
> The user guide also describes this as a comma-separated list of colors
> or ranges, with ranges written as hyphen-separated inclusive intervals.
> I don't see an empty-token production there.

What you quote is insufficient to determine: COLOR may be allowed to be
<nothing>. Iirc the reasoning went in particular towards a range with
merely the upper end specified being something (halfway) meaningful.

Jan

On Mon, May 18, 2026 at 9:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 18.05.2026 08:42, Mykola Kvach wrote:
> > On Mon, May 18, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 16.05.2026 17:03, Mykola Kvach wrote:
> >>> This small series fixes two issues in parse_color_config().
> >>>
> >>> The first patch makes parse failures leave the caller-visible color count
> >>> at zero.  This prevents a rejected command-line value from leaving a
> >>> partially parsed configuration behind for later init paths to consume.
> >>>
> >>> The second patch rejects empty color tokens.  Previously, delimiters in
> >>> places where a color value was expected could be interpreted as color 0,
> >>> because simple_strtoul() returns zero without advancing the input pointer.
> >>> The patch checks that each parsed color value consumed input.  It also
> >>> adds the missing newline to the DT color parsing error message.
> >>>
> >>> Mykola Kvach (2):
> >>>   xen/common: llc-coloring: clear color count on parse failure
> >>>   xen/common: llc-coloring: reject empty color tokens
> >>
> >> For both of these, a question which isn't even considered in the reasoning
> >> is whether the present behavior may be intentional. Especially for the 2nd
> >> ISTR Stefano(?) not so long ago indicating that the behavior is indeed
> >> intended to be this way. That may have been somewhere on Matrix rather than
> >> on the list, though.
> >
> > Thank you for pointing this out.
> >
> > For the first patch, my reasoning was that returning an error while
> > leaving a partially parsed caller-visible color count behind looks
> > surprising. If the value is rejected, I think later init paths should
> > not be able to consume the partially parsed state.
> >
> > For the second patch, my reasoning was that the current behavior looks
> > accidental rather than an intentional extension of the syntax.
>
> That was my impression as well, hence why I had raised the question back
> then.
>
> > The parser comment says:
> >
> > COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
> > RANGE ::= COLOR-COLOR
> >
> > The user guide also describes this as a comma-separated list of colors
> > or ranges, with ranges written as hyphen-separated inclusive intervals.
> > I don't see an empty-token production there.
>
> What you quote is insufficient to determine: COLOR may be allowed to be
> <nothing>. Iirc the reasoning went in particular towards a range with
> merely the upper end specified being something (halfway) meaningful.

Right, I see your point.

The grammar does not define COLOR explicitly, so it does not by itself
prove that an empty token is invalid. I was implicitly reading COLOR as
a numeric color value, partly because all examples seem to use numeric
values, but I agree that this is not stated there.

If this behavior is intentional, then I agree the second patch should
not go in as-is.

Best regards,
Mykola

On 18.05.2026 09:01, Mykola Kvach wrote:
> On Mon, May 18, 2026 at 9:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 18.05.2026 08:42, Mykola Kvach wrote:
>>> On Mon, May 18, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 16.05.2026 17:03, Mykola Kvach wrote:
>>>>> This small series fixes two issues in parse_color_config().
>>>>>
>>>>> The first patch makes parse failures leave the caller-visible color count
>>>>> at zero.  This prevents a rejected command-line value from leaving a
>>>>> partially parsed configuration behind for later init paths to consume.
>>>>>
>>>>> The second patch rejects empty color tokens.  Previously, delimiters in
>>>>> places where a color value was expected could be interpreted as color 0,
>>>>> because simple_strtoul() returns zero without advancing the input pointer.
>>>>> The patch checks that each parsed color value consumed input.  It also
>>>>> adds the missing newline to the DT color parsing error message.
>>>>>
>>>>> Mykola Kvach (2):
>>>>>   xen/common: llc-coloring: clear color count on parse failure
>>>>>   xen/common: llc-coloring: reject empty color tokens
>>>>
>>>> For both of these, a question which isn't even considered in the reasoning
>>>> is whether the present behavior may be intentional. Especially for the 2nd
>>>> ISTR Stefano(?) not so long ago indicating that the behavior is indeed
>>>> intended to be this way. That may have been somewhere on Matrix rather than
>>>> on the list, though.
>>>
>>> Thank you for pointing this out.
>>>
>>> For the first patch, my reasoning was that returning an error while
>>> leaving a partially parsed caller-visible color count behind looks
>>> surprising. If the value is rejected, I think later init paths should
>>> not be able to consume the partially parsed state.
>>>
>>> For the second patch, my reasoning was that the current behavior looks
>>> accidental rather than an intentional extension of the syntax.
>>
>> That was my impression as well, hence why I had raised the question back
>> then.
>>
>>> The parser comment says:
>>>
>>> COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
>>> RANGE ::= COLOR-COLOR
>>>
>>> The user guide also describes this as a comma-separated list of colors
>>> or ranges, with ranges written as hyphen-separated inclusive intervals.
>>> I don't see an empty-token production there.
>>
>> What you quote is insufficient to determine: COLOR may be allowed to be
>> <nothing>. Iirc the reasoning went in particular towards a range with
>> merely the upper end specified being something (halfway) meaningful.
> 
> Right, I see your point.
> 
> The grammar does not define COLOR explicitly, so it does not by itself
> prove that an empty token is invalid. I was implicitly reading COLOR as
> a numeric color value, partly because all examples seem to use numeric
> values, but I agree that this is not stated there.
> 
> If this behavior is intentional, then I agree the second patch should
> not go in as-is.

Just to mention: Something like "-5" won't be interpreted as "0-5" even
right now. Instead it's taken as a single color with value -5U, afaict.

Jan

On Mon, May 18, 2026 at 10:06 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 18.05.2026 09:01, Mykola Kvach wrote:
> > On Mon, May 18, 2026 at 9:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 18.05.2026 08:42, Mykola Kvach wrote:
> >>> On Mon, May 18, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 16.05.2026 17:03, Mykola Kvach wrote:
> >>>>> This small series fixes two issues in parse_color_config().
> >>>>>
> >>>>> The first patch makes parse failures leave the caller-visible color count
> >>>>> at zero.  This prevents a rejected command-line value from leaving a
> >>>>> partially parsed configuration behind for later init paths to consume.
> >>>>>
> >>>>> The second patch rejects empty color tokens.  Previously, delimiters in
> >>>>> places where a color value was expected could be interpreted as color 0,
> >>>>> because simple_strtoul() returns zero without advancing the input pointer.
> >>>>> The patch checks that each parsed color value consumed input.  It also
> >>>>> adds the missing newline to the DT color parsing error message.
> >>>>>
> >>>>> Mykola Kvach (2):
> >>>>>   xen/common: llc-coloring: clear color count on parse failure
> >>>>>   xen/common: llc-coloring: reject empty color tokens
> >>>>
> >>>> For both of these, a question which isn't even considered in the reasoning
> >>>> is whether the present behavior may be intentional. Especially for the 2nd
> >>>> ISTR Stefano(?) not so long ago indicating that the behavior is indeed
> >>>> intended to be this way. That may have been somewhere on Matrix rather than
> >>>> on the list, though.
> >>>
> >>> Thank you for pointing this out.
> >>>
> >>> For the first patch, my reasoning was that returning an error while
> >>> leaving a partially parsed caller-visible color count behind looks
> >>> surprising. If the value is rejected, I think later init paths should
> >>> not be able to consume the partially parsed state.
> >>>
> >>> For the second patch, my reasoning was that the current behavior looks
> >>> accidental rather than an intentional extension of the syntax.
> >>
> >> That was my impression as well, hence why I had raised the question back
> >> then.
> >>
> >>> The parser comment says:
> >>>
> >>> COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
> >>> RANGE ::= COLOR-COLOR
> >>>
> >>> The user guide also describes this as a comma-separated list of colors
> >>> or ranges, with ranges written as hyphen-separated inclusive intervals.
> >>> I don't see an empty-token production there.
> >>
> >> What you quote is insufficient to determine: COLOR may be allowed to be
> >> <nothing>. Iirc the reasoning went in particular towards a range with
> >> merely the upper end specified being something (halfway) meaningful.
> >
> > Right, I see your point.
> >
> > The grammar does not define COLOR explicitly, so it does not by itself
> > prove that an empty token is invalid. I was implicitly reading COLOR as
> > a numeric color value, partly because all examples seem to use numeric
> > values, but I agree that this is not stated there.
> >
> > If this behavior is intentional, then I agree the second patch should
> > not go in as-is.
>
> Just to mention: Something like "-5" won't be interpreted as "0-5" even
> right now. Instead it's taken as a single color with value -5U, afaict.

I don't think that is what happens with the current parser.

I tested this without the patches from this series:

(XEN) Command line: dom0_mem=2048M console=dtuart dtuart=serial0
(XEN) loglvl=all console_timestamps=boot llc-coloring=on
(XEN) xen-llc-colors=-5
...
(XEN) LLC coloring info:
(XEN) Number of LLC colors supported: 32
(XEN) Xen LLC colors (6): { 0-5 }

So "-5" is currently interpreted as "0-5", not as a single color with
value -5U.

That seems to happen because simple_strtoul() does not consume the
leading '-', so start remains 0 and the parser then takes the range
path.

Best regards,
Mykola

On 18.05.2026 09:19, Mykola Kvach wrote:
> On Mon, May 18, 2026 at 10:06 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 18.05.2026 09:01, Mykola Kvach wrote:
>>> On Mon, May 18, 2026 at 9:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 18.05.2026 08:42, Mykola Kvach wrote:
>>>>> On Mon, May 18, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 16.05.2026 17:03, Mykola Kvach wrote:
>>>>>>> This small series fixes two issues in parse_color_config().
>>>>>>>
>>>>>>> The first patch makes parse failures leave the caller-visible color count
>>>>>>> at zero.  This prevents a rejected command-line value from leaving a
>>>>>>> partially parsed configuration behind for later init paths to consume.
>>>>>>>
>>>>>>> The second patch rejects empty color tokens.  Previously, delimiters in
>>>>>>> places where a color value was expected could be interpreted as color 0,
>>>>>>> because simple_strtoul() returns zero without advancing the input pointer.
>>>>>>> The patch checks that each parsed color value consumed input.  It also
>>>>>>> adds the missing newline to the DT color parsing error message.
>>>>>>>
>>>>>>> Mykola Kvach (2):
>>>>>>>   xen/common: llc-coloring: clear color count on parse failure
>>>>>>>   xen/common: llc-coloring: reject empty color tokens
>>>>>>
>>>>>> For both of these, a question which isn't even considered in the reasoning
>>>>>> is whether the present behavior may be intentional. Especially for the 2nd
>>>>>> ISTR Stefano(?) not so long ago indicating that the behavior is indeed
>>>>>> intended to be this way. That may have been somewhere on Matrix rather than
>>>>>> on the list, though.
>>>>>
>>>>> Thank you for pointing this out.
>>>>>
>>>>> For the first patch, my reasoning was that returning an error while
>>>>> leaving a partially parsed caller-visible color count behind looks
>>>>> surprising. If the value is rejected, I think later init paths should
>>>>> not be able to consume the partially parsed state.
>>>>>
>>>>> For the second patch, my reasoning was that the current behavior looks
>>>>> accidental rather than an intentional extension of the syntax.
>>>>
>>>> That was my impression as well, hence why I had raised the question back
>>>> then.
>>>>
>>>>> The parser comment says:
>>>>>
>>>>> COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
>>>>> RANGE ::= COLOR-COLOR
>>>>>
>>>>> The user guide also describes this as a comma-separated list of colors
>>>>> or ranges, with ranges written as hyphen-separated inclusive intervals.
>>>>> I don't see an empty-token production there.
>>>>
>>>> What you quote is insufficient to determine: COLOR may be allowed to be
>>>> <nothing>. Iirc the reasoning went in particular towards a range with
>>>> merely the upper end specified being something (halfway) meaningful.
>>>
>>> Right, I see your point.
>>>
>>> The grammar does not define COLOR explicitly, so it does not by itself
>>> prove that an empty token is invalid. I was implicitly reading COLOR as
>>> a numeric color value, partly because all examples seem to use numeric
>>> values, but I agree that this is not stated there.
>>>
>>> If this behavior is intentional, then I agree the second patch should
>>> not go in as-is.
>>
>> Just to mention: Something like "-5" won't be interpreted as "0-5" even
>> right now. Instead it's taken as a single color with value -5U, afaict.
> 
> I don't think that is what happens with the current parser.
> 
> I tested this without the patches from this series:
> 
> (XEN) Command line: dom0_mem=2048M console=dtuart dtuart=serial0
> (XEN) loglvl=all console_timestamps=boot llc-coloring=on
> (XEN) xen-llc-colors=-5
> ...
> (XEN) LLC coloring info:
> (XEN) Number of LLC colors supported: 32
> (XEN) Xen LLC colors (6): { 0-5 }
> 
> So "-5" is currently interpreted as "0-5", not as a single color with
> value -5U.
> 
> That seems to happen because simple_strtoul() does not consume the
> leading '-', so start remains 0 and the parser then takes the range
> path.

Oh, I didn't recall this incompatibility with strtoul(). We also don't
permit a leading + there. IOW the behavior of the color parsing would
change if we made the functions (more) compatible with the standard.

Jan

On Mon, May 18, 2026 at 10:43 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 18.05.2026 09:19, Mykola Kvach wrote:
> > On Mon, May 18, 2026 at 10:06 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 18.05.2026 09:01, Mykola Kvach wrote:
> >>> On Mon, May 18, 2026 at 9:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 18.05.2026 08:42, Mykola Kvach wrote:
> >>>>> On Mon, May 18, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>> On 16.05.2026 17:03, Mykola Kvach wrote:
> >>>>>>> This small series fixes two issues in parse_color_config().
> >>>>>>>
> >>>>>>> The first patch makes parse failures leave the caller-visible color count
> >>>>>>> at zero.  This prevents a rejected command-line value from leaving a
> >>>>>>> partially parsed configuration behind for later init paths to consume.
> >>>>>>>
> >>>>>>> The second patch rejects empty color tokens.  Previously, delimiters in
> >>>>>>> places where a color value was expected could be interpreted as color 0,
> >>>>>>> because simple_strtoul() returns zero without advancing the input pointer.
> >>>>>>> The patch checks that each parsed color value consumed input.  It also
> >>>>>>> adds the missing newline to the DT color parsing error message.
> >>>>>>>
> >>>>>>> Mykola Kvach (2):
> >>>>>>>   xen/common: llc-coloring: clear color count on parse failure
> >>>>>>>   xen/common: llc-coloring: reject empty color tokens
> >>>>>>
> >>>>>> For both of these, a question which isn't even considered in the reasoning
> >>>>>> is whether the present behavior may be intentional. Especially for the 2nd
> >>>>>> ISTR Stefano(?) not so long ago indicating that the behavior is indeed
> >>>>>> intended to be this way. That may have been somewhere on Matrix rather than
> >>>>>> on the list, though.
> >>>>>
> >>>>> Thank you for pointing this out.
> >>>>>
> >>>>> For the first patch, my reasoning was that returning an error while
> >>>>> leaving a partially parsed caller-visible color count behind looks
> >>>>> surprising. If the value is rejected, I think later init paths should
> >>>>> not be able to consume the partially parsed state.
> >>>>>
> >>>>> For the second patch, my reasoning was that the current behavior looks
> >>>>> accidental rather than an intentional extension of the syntax.
> >>>>
> >>>> That was my impression as well, hence why I had raised the question back
> >>>> then.
> >>>>
> >>>>> The parser comment says:
> >>>>>
> >>>>> COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
> >>>>> RANGE ::= COLOR-COLOR
> >>>>>
> >>>>> The user guide also describes this as a comma-separated list of colors
> >>>>> or ranges, with ranges written as hyphen-separated inclusive intervals.
> >>>>> I don't see an empty-token production there.
> >>>>
> >>>> What you quote is insufficient to determine: COLOR may be allowed to be
> >>>> <nothing>. Iirc the reasoning went in particular towards a range with
> >>>> merely the upper end specified being something (halfway) meaningful.
> >>>
> >>> Right, I see your point.
> >>>
> >>> The grammar does not define COLOR explicitly, so it does not by itself
> >>> prove that an empty token is invalid. I was implicitly reading COLOR as
> >>> a numeric color value, partly because all examples seem to use numeric
> >>> values, but I agree that this is not stated there.
> >>>
> >>> If this behavior is intentional, then I agree the second patch should
> >>> not go in as-is.
> >>
> >> Just to mention: Something like "-5" won't be interpreted as "0-5" even
> >> right now. Instead it's taken as a single color with value -5U, afaict.
> >
> > I don't think that is what happens with the current parser.
> >
> > I tested this without the patches from this series:
> >
> > (XEN) Command line: dom0_mem=2048M console=dtuart dtuart=serial0
> > (XEN) loglvl=all console_timestamps=boot llc-coloring=on
> > (XEN) xen-llc-colors=-5
> > ...
> > (XEN) LLC coloring info:
> > (XEN) Number of LLC colors supported: 32
> > (XEN) Xen LLC colors (6): { 0-5 }
> >
> > So "-5" is currently interpreted as "0-5", not as a single color with
> > value -5U.
> >
> > That seems to happen because simple_strtoul() does not consume the
> > leading '-', so start remains 0 and the parser then takes the range
> > path.
>
> Oh, I didn't recall this incompatibility with strtoul(). We also don't
> permit a leading + there. IOW the behavior of the color parsing would
> change if we made the functions (more) compatible with the standard.

Right.

The patch makes this explicit in one direction: a COLOR must consume
input. If "-N" is intended to be valid syntax, then I think it should be
handled as a separate explicit case, rather than relying on
simple_strtoul() stopping at '-'.

Best regards,
Mykola