Re: [PATCH] User definable terminfo support

[Marco Gerards <mgerards@xs4all.nl>]

Omniflux <omniflux+lists@omniflux.com> writes:

> Marco Gerards wrote:
>> Here is the review of the code that I promised.  Please see my other
>> email about generic comments.
>
> What is the subject line of this message? I sadly do not read all
> messages on this list.

You just answered it, so I guess you found it. ;-)

>> You can put the string on a new line.  If the string is still too long
>> you can make two strings out of it, like:
>> grub_error (FOO,
>>             "bar"
>>             "baz");
>
> grub_error (FOO, "bar" "baz");
> No operator or separator between strings?

Right.  That will concatenate them and result in the string "barbaz".

>>>+          ti->name   = grub_strdup (args[0]);
>> Typo.
>
> Can you be more specific? This looks correct to me...

Can you change that to:

ti->name = grub_strdup (args[0]);

(So only one space before `=')

>>>+static void
>>>+setup_defaults (void)
>>>+{
>>>+  terminfo_t *ti;
>>>+
>>>+  ti = (terminfo_t *) grub_malloc (sizeof (terminfo_t));
>>>+  /* Do I need to test if malloc succeeded here? What do I do if it did not? */
>> Good one :-)
>> In that case default could be set to NULL, if that is allowed?
>
> I think in this case, there will be many other problems. I think the
> best thing in this case will be, as you suggest, to set the definition
> to NULL and have all terminfo functions return their strings
> unmodified.
>
> The only consequence to this I can think of is loss of menu support on
> terminals which depend on terminfo (serial), and should be a rare case
> even there.

Another thing you could do is not malloc'ing memory, but using a
pointer to a pre-defined struct instead.  When free'ing you need a
special case to check if it is this pre-defined struct.

--
Marco