[PATCH] kconfig: display an error message when aborting

[PATCH] kconfig: display an error message when aborting

[Mike Frysinger]

If the Kconfig option causes an open() failure (like one that starts with
an underscore), there should be an error message shown since we're going
to be exiting with an error code.  Otherwise, the reason for the failure
can really only be diagnosed with strace or something similar.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 scripts/kconfig/confdata.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index b55e72f..e2644b4 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -640,6 +640,8 @@ static int conf_split_config(void)
 		fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
 		if (fd == -1) {
 			if (errno != ENOENT) {
+				conf_warning("sym '%s' with path '%s': %s",
+					     sym->name, path, strerror(errno));
 				res = 1;
 				break;
 			}
-- 
1.6.5.3

Re: [PATCH] kconfig: display an error message when aborting

[Michal Marek]

On 25.11.2009 05:48, Mike Frysinger wrote:
> If the Kconfig option causes an open() failure (like one that starts with
> an underscore), there should be an error message shown since we're going
> to be exiting with an error code.  Otherwise, the reason for the failure
> can really only be diagnosed with strace or something similar.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  scripts/kconfig/confdata.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index b55e72f..e2644b4 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -640,6 +640,8 @@ static int conf_split_config(void)
>  		fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
>  		if (fd == -1) {
>  			if (errno != ENOENT) {
> +				conf_warning("sym '%s' with path '%s': %s",
> +					     sym->name, path, strerror(errno));
>  				res = 1;
>  				break;
>  			}

I agree that there definitely needs to be some error reporting (and not
only here but in many more places, look e.g. at conf_write() or
conf_write_autoconf()), but why use conf_warning() for this? It will
prefix the error message with "include/config/auto.conf:<last lineno>",
which has nothing to do with the path that could not be opened.

Michal

Re: [PATCH] kconfig: display an error message when aborting

[Mike Frysinger]

2009/11/26 Michal Marek
> On 25.11.2009 05:48, Mike Frysinger wrote:
>> If the Kconfig option causes an open() failure (like one that starts with
>> an underscore), there should be an error message shown since we're going
>> to be exiting with an error code.  Otherwise, the reason for the failure
>> can really only be diagnosed with strace or something similar.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>>  scripts/kconfig/confdata.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>> index b55e72f..e2644b4 100644
>> --- a/scripts/kconfig/confdata.c
>> +++ b/scripts/kconfig/confdata.c
>> @@ -640,6 +640,8 @@ static int conf_split_config(void)
>>               fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
>>               if (fd == -1) {
>>                       if (errno != ENOENT) {
>> +                             conf_warning("sym '%s' with path '%s': %s",
>> +                                          sym->name, path, strerror(errno));
>>                               res = 1;
>>                               break;
>>                       }
>
> I agree that there definitely needs to be some error reporting (and not
> only here but in many more places, look e.g. at conf_write() or
> conf_write_autoconf()), but why use conf_warning() for this? It will
> prefix the error message with "include/config/auto.conf:<last lineno>",
> which has nothing to do with the path that could not be opened.

no it doesnt.  it prefixes the config file name which i think is relevant.
    .config:1871:warning: sym '_BF548' with path '/bf548.h': Permission denied
-mike

Re: [PATCH] kconfig: display an error message when aborting

[Michal Marek]

Mike Frysinger napsal(a):
> 2009/11/26 Michal Marek
>> On 25.11.2009 05:48, Mike Frysinger wrote:
>>> If the Kconfig option causes an open() failure (like one that starts with
>>> an underscore), there should be an error message shown since we're going
>>> to be exiting with an error code.  Otherwise, the reason for the failure
>>> can really only be diagnosed with strace or something similar.
>>>
>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>> ---
>>>  scripts/kconfig/confdata.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>>> index b55e72f..e2644b4 100644
>>> --- a/scripts/kconfig/confdata.c
>>> +++ b/scripts/kconfig/confdata.c
>>> @@ -640,6 +640,8 @@ static int conf_split_config(void)
>>>               fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
>>>               if (fd == -1) {
>>>                       if (errno != ENOENT) {
>>> +                             conf_warning("sym '%s' with path '%s': %s",
>>> +                                          sym->name, path, strerror(errno));
>>>                               res = 1;
>>>                               break;
>>>                       }
>> I agree that there definitely needs to be some error reporting (and not
>> only here but in many more places, look e.g. at conf_write() or
>> conf_write_autoconf()), but why use conf_warning() for this? It will
>> prefix the error message with "include/config/auto.conf:<last lineno>",
>> which has nothing to do with the path that could not be opened.
> 
> no it doesnt.  it prefixes the config file name which i think is relevant.
>     .config:1871:warning: sym '_BF548' with path '/bf548.h': Permission denied

Well, it prints either ".config" or "include/config/auto.conf",
depending whether there was a successful silentoldconfig pass before and
the latter file exists. But the number is the number of lines of the
respective file.

Michal

Re: [PATCH] kconfig: display an error message when aborting

[Mike Frysinger]

On Thu, Nov 26, 2009 at 15:07, Michal Marek wrote:
> Mike Frysinger napsal(a):
>> 2009/11/26 Michal Marek
>>> On 25.11.2009 05:48, Mike Frysinger wrote:
>>>> If the Kconfig option causes an open() failure (like one that starts with
>>>> an underscore), there should be an error message shown since we're going
>>>> to be exiting with an error code.  Otherwise, the reason for the failure
>>>> can really only be diagnosed with strace or something similar.
>>>>
>>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>>> ---
>>>>  scripts/kconfig/confdata.c |    2 ++
>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>>>> index b55e72f..e2644b4 100644
>>>> --- a/scripts/kconfig/confdata.c
>>>> +++ b/scripts/kconfig/confdata.c
>>>> @@ -640,6 +640,8 @@ static int conf_split_config(void)
>>>>               fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
>>>>               if (fd == -1) {
>>>>                       if (errno != ENOENT) {
>>>> +                             conf_warning("sym '%s' with path '%s': %s",
>>>> +                                          sym->name, path, strerror(errno));
>>>>                               res = 1;
>>>>                               break;
>>>>                       }
>>>
>>> I agree that there definitely needs to be some error reporting (and not
>>> only here but in many more places, look e.g. at conf_write() or
>>> conf_write_autoconf()), but why use conf_warning() for this? It will
>>> prefix the error message with "include/config/auto.conf:<last lineno>",
>>> which has nothing to do with the path that could not be opened.
>>
>> no it doesnt.  it prefixes the config file name which i think is relevant.
>>     .config:1871:warning: sym '_BF548' with path '/bf548.h': Permission denied
>
> Well, it prints either ".config" or "include/config/auto.conf",
> depending whether there was a successful silentoldconfig pass before and
> the latter file exists. But the number is the number of lines of the
> respective file.

if you want to improve kconfig's error reporing in general, have at
it, but conf_warning() appears to be the standard in confdata.c for
reporting errors/warnings to stderr.  your complaint applies to just
about every usage of conf_warning() in confdata.c.  i'm not going to
create my own format and fprintf() directly to stderr.
-mike

Re: [PATCH] kconfig: display an error message when aborting

[Michal Marek]

Mike Frysinger napsal(a):
> On Thu, Nov 26, 2009 at 15:07, Michal Marek wrote:
>> Mike Frysinger napsal(a):
>>> 2009/11/26 Michal Marek
>>>> On 25.11.2009 05:48, Mike Frysinger wrote:
>>>>> If the Kconfig option causes an open() failure (like one that starts with
>>>>> an underscore), there should be an error message shown since we're going
>>>>> to be exiting with an error code.  Otherwise, the reason for the failure
>>>>> can really only be diagnosed with strace or something similar.
>>>>>
>>>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>>>> ---
>>>>>  scripts/kconfig/confdata.c |    2 ++
>>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>>>>> index b55e72f..e2644b4 100644
>>>>> --- a/scripts/kconfig/confdata.c
>>>>> +++ b/scripts/kconfig/confdata.c
>>>>> @@ -640,6 +640,8 @@ static int conf_split_config(void)
>>>>>               fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
>>>>>               if (fd == -1) {
>>>>>                       if (errno != ENOENT) {
>>>>> +                             conf_warning("sym '%s' with path '%s': %s",
>>>>> +                                          sym->name, path, strerror(errno));
>>>>>                               res = 1;
>>>>>                               break;
>>>>>                       }
>>>> I agree that there definitely needs to be some error reporting (and not
>>>> only here but in many more places, look e.g. at conf_write() or
>>>> conf_write_autoconf()), but why use conf_warning() for this? It will
>>>> prefix the error message with "include/config/auto.conf:<last lineno>",
>>>> which has nothing to do with the path that could not be opened.
>>> no it doesnt.  it prefixes the config file name which i think is relevant.
>>>     .config:1871:warning: sym '_BF548' with path '/bf548.h': Permission denied
>> Well, it prints either ".config" or "include/config/auto.conf",
>> depending whether there was a successful silentoldconfig pass before and
>> the latter file exists. But the number is the number of lines of the
>> respective file.
> 
> if you want to improve kconfig's error reporing in general, have at
> it,

I'll write something if time permits.

> but conf_warning() appears to be the standard in confdata.c for
> reporting errors/warnings to stderr.  your complaint applies to just
> about every usage of conf_warning() in confdata.c.  i'm not going to
> create my own format and fprintf() directly to stderr.

No. conf_warning() does the right thing when called from within
conf_read_simple(), which maintains the conf_filename and conf_lineno
variables. That's the point I was trying to make.

Michal