Re: [libvirt] [PATCH v3 1/8] vsh: Fix NULL dereference leading to SIGSEGV if a command is missing '.info'

2016-09-19 Thread Erik Skultety 
On 16/09/16 14:19, Ján Tomko wrote:
> On Fri, Sep 16, 2016 at 12:50:38PM +0200, Erik Skultety wrote:
>> Signed-off-by: Erik Skultety 
>> ---
>> tools/vsh.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Is there a use-case for a command without info?
> 
> I think all the commands should have a description and we should enforce
> that in the test suite.
> 

Well, if you look at domhostname command, it does provide an empty
string for description, since there already is a help string that says
everything important. So I think description does not need to be
enforced in the test suite. But we sure should enforce checking for
help. Command without help is not much of a help, is it...

Erik

>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index 4ee472c..3772d92 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -695,7 +695,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
>> }
>> fputc('\n', stdout);
>>
>> -if (desc[0]) {
>> +if (desc && *desc) {
> 
> Or maybe report an error here if the description is missing?
> 
> Jan
>> /* Print the description only if it's not empty.  */
>> fputs(_("\n  DESCRIPTION\n"), stdout);
>> fprintf(stdout, "%s\n", _(desc));
>> -- 
>> 2.5.5
>>
>> -- 
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/8] vsh: Fix NULL dereference leading to SIGSEGV if a command is missing '.info'

2016-09-16 Thread Erik Skultety 
On 16/09/16 14:19, Ján Tomko wrote:
> On Fri, Sep 16, 2016 at 12:50:38PM +0200, Erik Skultety wrote:
>> Signed-off-by: Erik Skultety 
>> ---
>> tools/vsh.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Is there a use-case for a command without info?
> 
> I think all the commands should have a description and we should enforce
> that in the test suite.
> 

Well, if I call vshCmddefCheckInternals from the self-test as suggested
by 4/8, then we could also go your suggested way here as well. I fixed
the rest and will push it in a while. I'll send an updated version of
this later.

Thanks,
Erik

>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index 4ee472c..3772d92 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -695,7 +695,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
>> }
>> fputc('\n', stdout);
>>
>> -if (desc[0]) {
>> +if (desc && *desc) {
> 
> Or maybe report an error here if the description is missing?
> 
> Jan
>> /* Print the description only if it's not empty.  */
>> fputs(_("\n  DESCRIPTION\n"), stdout);
>> fprintf(stdout, "%s\n", _(desc));
>> -- 
>> 2.5.5
>>
>> -- 
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/8] vsh: Fix NULL dereference leading to SIGSEGV if a command is missing '.info'

2016-09-16 Thread Ján Tomko 

On Fri, Sep 16, 2016 at 12:50:38PM +0200, Erik Skultety wrote:

Signed-off-by: Erik Skultety 
---
tools/vsh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Is there a use-case for a command without info?

I think all the commands should have a description and we should enforce
that in the test suite.


diff --git a/tools/vsh.c b/tools/vsh.c
index 4ee472c..3772d92 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -695,7 +695,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
}
fputc('\n', stdout);

-if (desc[0]) {
+if (desc && *desc) {


Or maybe report an error here if the description is missing?

Jan

/* Print the description only if it's not empty.  */
fputs(_("\n  DESCRIPTION\n"), stdout);
fprintf(stdout, "%s\n", _(desc));
--
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 1/8] vsh: Fix NULL dereference leading to SIGSEGV if a command is missing '.info'

2016-09-16 Thread Erik Skultety 
Signed-off-by: Erik Skultety 
---
 tools/vsh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 4ee472c..3772d92 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -695,7 +695,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
 }
 fputc('\n', stdout);
 
-if (desc[0]) {
+if (desc && *desc) {
 /* Print the description only if it's not empty.  */
 fputs(_("\n  DESCRIPTION\n"), stdout);
 fprintf(stdout, "%s\n", _(desc));
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list