All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Testov <vladimir.testov@rosalab.ru>
To: grub-devel@gnu.org
Subject: Re: [PATCH] gfxterm: check elements' properties and hadle errors.
Date: Thu, 07 Mar 2013 10:29:14 +0400	[thread overview]
Message-ID: <9805474.MFZGl5BBPZ@icedphoenix> (raw)

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

>Again - what problem are you trying to solve? I added unknown option
>to theme element and it was loaded and displayed just fine. Why
>aborting theme loading in this case is better?

There is possibility that we can misprint some option. Or write some wrong value for some existing option.
I think it's better to know about it - so we can quickly find the place where we are wrong.

>May be you should be less hostile to simple questions? I take it you
>are expert in GRUB2 themes. I'm not and I'm trying to learn. OK?

Sorry, if I've insulted you. That wasn't my intension.

>You conveniently ignored comment about losing original error.
The original error will be displayed anyway, because it's called via grub_error function.
grub_errno in that case is only an indicator that everything went all right or something went wrong.
Returning error state (from component's modules) doesn't change grub_errno until we call grub_error in theme_loader.
In current state return value isn't analized. And I think it's not good.
(if theme_loaders checks global options and component's names - why shouldn't it also check for component's options)

I could think about more complicated hadling of return value.
If it is GRUB_ERR_IO then either information message about the error has already been shown
(don't see the possibility of the fact right now)
or we've faced some non-existing option (or option with a misprint)
So I could check for other types of error and hadle them more properly.

grub_errno in that case means error code, returned after trying to set a particular value to a particular option.
so if return code is GRUB_ERR_IO then there is a problem with option name
otherwise there is a problem with a value (e.g. we tried to set integer value to a option, which accepts string value.)

If you concerns are about right that - I can slightly remake the patch to show more detailed information about the error.

Cheers.
-- 
With best regards,
_______________________________
Vladimir Testov, ROSA Laboratory.
www.rosalab.ru

[-- Attachment #2: Type: text/html, Size: 9626 bytes --]

             reply	other threads:[~2013-03-07  6:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-07  6:29 Vladimir Testov [this message]
  -- strict thread matches above, loose matches on Subject: below --
2013-03-07  7:46 [PATCH] gfxterm: check elements' properties and hadle errors Vladimir Testov
2013-03-07  4:22 Vladimir Testov
2013-03-07  6:03 ` Andrey Borzenkov
2013-03-04 11:27 Vladimir Testov
2013-03-06 18:05 ` Andrey Borzenkov
2013-03-07  7:31 ` Vladimir 'φ-coder/phcoder' Serbinenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9805474.MFZGl5BBPZ@icedphoenix \
    --to=vladimir.testov@rosalab.ru \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.