Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] kconfig: display an error message when aborting

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


vapier at gentoo

Nov 24, 2009, 8:48 PM

Post #1 of 6 (146 views)
Permalink
[PATCH] kconfig: display an error message when aborting

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 [at] gentoo>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mmarek at suse

Nov 26, 2009, 3:15 AM

Post #2 of 6 (134 views)
Permalink
Re: [PATCH] kconfig: display an error message when aborting [In reply to]

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 [at] gentoo>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


vapier.adi at gmail

Nov 26, 2009, 11:03 AM

Post #3 of 6 (129 views)
Permalink
Re: [PATCH] kconfig: display an error message when aborting [In reply to]

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 [at] gentoo>
>> ---
>>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mmarek at suse

Nov 26, 2009, 12:07 PM

Post #4 of 6 (129 views)
Permalink
Re: [PATCH] kconfig: display an error message when aborting [In reply to]

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 [at] gentoo>
>>> ---
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


vapier.adi at gmail

Nov 26, 2009, 12:12 PM

Post #5 of 6 (128 views)
Permalink
Re: [PATCH] kconfig: display an error message when aborting [In reply to]

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 [at] gentoo>
>>>> ---
>>>>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mmarek at suse

Nov 26, 2009, 12:24 PM

Post #6 of 6 (125 views)
Permalink
Re: [PATCH] kconfig: display an error message when aborting [In reply to]

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 [at] gentoo>
>>>>> ---
>>>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.