bug in col.read()

2 views
Skip to first unread message

Roger Kaufman

unread,
Nov 7, 2023, 11:14:05 AM11/7/23
to anti...@googlegroups.com
Hi Adrian,

I'm working on a color code idea and I ran across an obscure bug.

In off_color these options are unused. I checked them all.

Singles:
e
f
m
v

Pairs:
bB
hH  <== causes segfault in off_color
iI
oO
qQ
tT
wW
xX <== alone is valid color of invisible (hexadecimal format)
yY
zZ

When I try 'off_color tet -f h/H' it segfaults.

I traced it to Color::read_hsva_vals which tries to access the values
after H but there are none the array is size 0.

It could error out, but if so then h or H would be an invalid color. To
make it act like x/X alone the fix could be

// if h or H appeared alone, all ranges are zero
if (!vals.size())
    vals = {0, 0, 0, 0};

If no values are given anything else would change to an unexpected
color. What do you think?

Roger

Adrian Rossiter

unread,
Nov 7, 2023, 1:11:19 PM11/7/23
to anti...@googlegroups.com
Hi Roger

On Tue, 7 Nov 2023, Roger Kaufman wrote:
> hH  <== causes segfault in off_color
...
> xX <== alone is valid color of invisible (hexadecimal format)
...
> I traced it to Color::read_hsva_vals which tries to access the values after H
> but there are none the array is size 0.
>
> It could error out, but if so then h or H would be an invalid color. To make
> it act like x/X alone the fix could be
>
> // if h or H appeared alone, all ranges are zero
> if (!vals.size())
>     vals = {0, 0, 0, 0};
>
> If no values are given anything else would change to an unexpected color.
> What do you think?

Good catch.

To me, "x" is more like a shorthand for "invisible", rather than being
of the form "xNNNNNN[NN]" without any numbers. In which case a bare "h"
(or "H") could just be an invalid colour, with no expectation that it
would be the invisible colour (the format of "x" is also already different
to the format of "h".)

The use of "x" and "X" for invisible might be undocumented. It could
be mentioned where "invisible" is mentioned in 'off_util -H col_names'

Adrian.
--
Adrian Rossiter
adr...@antiprism.com
http://antiprism.com/adrian

Roger Kaufman

unread,
Nov 7, 2023, 1:37:56 PM11/7/23
to anti...@googlegroups.com
Hi Adrian,

ok then the change is as simple as

Status Color::read_hsva_vals(const char *str)
{
    if (strlen(str) == 0)

to

    if (strlen(str) < 2)

This has the side effect of making h/H available for a color operation
if ever needed.

On 11/7/2023 1:11 PM, Adrian Rossiter wrote:
> The use of "x" and "X" for invisible might be undocumented. It could
> be mentioned where "invisible" is mentioned in 'off_util -H col_names'

...
resources. Other names: 'invisible' corresponds to transparent black,
and is used to indicate that display elements should be excluded from
display; 'none' is a colour with no colour data, setting an element to
this colour removes any colour data for the element

would be

and is used to indicate that display elements should be excluded from
display (a colour value of 'x' can also be used for invisible); 'none' is a
colour with no colour data, setting an element to...

The col_val help for the x format is

(e.g. orange is 'xFF8000')

could also be

(e.g. orange is 'xFF8000', 'x' alone, is transparent black and is a
shortcut for 'invisible')

Roger

Adrian Rossiter

unread,
Nov 7, 2023, 2:06:49 PM11/7/23
to anti...@googlegroups.com
Hi Roger

On Tue, 7 Nov 2023, Roger Kaufman wrote:
> ok then the change is as simple as
...
> and is used to indicate that display elements should be excluded from
> display (a colour value of 'x' can also be used for invisible); 'none' is a
> colour with no colour data, setting an element to...
...
> (e.g. orange is 'xFF8000', 'x' alone, is transparent black and is a shortcut
> for 'invisible')

All sounds good.
Reply all
Reply to author
Forward
0 new messages