* [PATCH] kconfig: display an error message when aborting
@ 2009-11-25 4:48 Mike Frysinger
2009-11-26 11:15 ` Michal Marek
0 siblings, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2009-11-25 4:48 UTC (permalink / raw)
To: linux-kbuild, Sam Ravnborg; +Cc: linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] kconfig: display an error message when aborting
2009-11-25 4:48 [PATCH] kconfig: display an error message when aborting Mike Frysinger
@ 2009-11-26 11:15 ` Michal Marek
2009-11-26 19:03 ` Mike Frysinger
0 siblings, 1 reply; 6+ messages in thread
From: Michal Marek @ 2009-11-26 11:15 UTC (permalink / raw)
To: Mike Frysinger; +Cc: linux-kbuild, Sam Ravnborg, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kconfig: display an error message when aborting
2009-11-26 11:15 ` Michal Marek
@ 2009-11-26 19:03 ` Mike Frysinger
2009-11-26 20:07 ` Michal Marek
0 siblings, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2009-11-26 19:03 UTC (permalink / raw)
To: Michal Marek; +Cc: linux-kbuild, Sam Ravnborg, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kconfig: display an error message when aborting
2009-11-26 19:03 ` Mike Frysinger
@ 2009-11-26 20:07 ` Michal Marek
2009-11-26 20:12 ` Mike Frysinger
0 siblings, 1 reply; 6+ messages in thread
From: Michal Marek @ 2009-11-26 20:07 UTC (permalink / raw)
To: Mike Frysinger; +Cc: linux-kbuild, Sam Ravnborg, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kconfig: display an error message when aborting
2009-11-26 20:07 ` Michal Marek
@ 2009-11-26 20:12 ` Mike Frysinger
2009-11-26 20:24 ` Michal Marek
0 siblings, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2009-11-26 20:12 UTC (permalink / raw)
To: Michal Marek; +Cc: linux-kbuild, Sam Ravnborg, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kconfig: display an error message when aborting
2009-11-26 20:12 ` Mike Frysinger
@ 2009-11-26 20:24 ` Michal Marek
0 siblings, 0 replies; 6+ messages in thread
From: Michal Marek @ 2009-11-26 20:24 UTC (permalink / raw)
To: Mike Frysinger; +Cc: linux-kbuild, Sam Ravnborg, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-11-26 20:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-25 4:48 [PATCH] kconfig: display an error message when aborting Mike Frysinger
2009-11-26 11:15 ` Michal Marek
2009-11-26 19:03 ` Mike Frysinger
2009-11-26 20:07 ` Michal Marek
2009-11-26 20:12 ` Mike Frysinger
2009-11-26 20:24 ` Michal Marek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).