diff mbox

[04/11] error: New QERR_INVALID_PARAMETER_VALUE

Message ID 1268929996-28763-5-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster March 18, 2010, 4:33 p.m. UTC 
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

Comments

Luiz Capitulino March 19, 2010, 7:49 p.m. UTC | #1 
On Thu, 18 Mar 2010 17:33:11 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qerror.c |    4 ++++
>  qerror.h |    3 +++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/qerror.c b/qerror.c
> index 9fb817e..97e8d4a 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
>          .desc      = "Invalid parameter type, expected: %(expected)",
>      },
>      {
> +        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
> +        .desc      = "Parameter '%(name)' expects %(expected)",
> +    },

 Would we need this error if we improve QERR_INVALID_PARAMETER to
accept an optional 'expects' parameter?

> +    {
>          .error_fmt = QERR_INVALID_PASSWORD,
>          .desc      = "Password incorrect",
>      },
> diff --git a/qerror.h b/qerror.h
> index 870cdc3..5625d54 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_INVALID_PARAMETER_TYPE \
>      "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
>  
> +#define QERR_INVALID_PARAMETER_VALUE \
> +    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
> +
>  #define QERR_INVALID_PASSWORD \
>      "{ 'class': 'InvalidPassword', 'data': {} }"
>
Markus Armbruster March 19, 2010, 9:05 p.m. UTC | #2 
Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Thu, 18 Mar 2010 17:33:11 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qerror.c |    4 ++++
>>  qerror.h |    3 +++
>>  2 files changed, 7 insertions(+), 0 deletions(-)
>> 
>> diff --git a/qerror.c b/qerror.c
>> index 9fb817e..97e8d4a 100644
>> --- a/qerror.c
>> +++ b/qerror.c
>> @@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
>>          .desc      = "Invalid parameter type, expected: %(expected)",
>>      },
>>      {
>> +        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
>> +        .desc      = "Parameter '%(name)' expects %(expected)",
>> +    },
>
>  Would we need this error if we improve QERR_INVALID_PARAMETER to
> accept an optional 'expects' parameter?

QERR_INVALID_PARAMETER_VALUE means "this value can't be accepted for
this parameter".

QERR_INVALID_PARAMETER means "this parameter can't be accepted,
regardless of value".  Pedantically speaking, that's a different error.
Pragmatically speaking, I doubt clients care much (but I'm the one who
doubts clients care much for differentiating errors in general, so I'm
biased).  Practically speaking, we can use a single error class
'InvalidParameter' if we don't care for the difference, but I figure we
still need separate QERR_ definitions, because we want different
human-readable error messages.

Want me to do anything here now?

>> +    {
>>          .error_fmt = QERR_INVALID_PASSWORD,
>>          .desc      = "Password incorrect",
>>      },
>> diff --git a/qerror.h b/qerror.h
>> index 870cdc3..5625d54 100644
>> --- a/qerror.h
>> +++ b/qerror.h
>> @@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
>>  #define QERR_INVALID_PARAMETER_TYPE \
>>      "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
>>  
>> +#define QERR_INVALID_PARAMETER_VALUE \
>> +    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
>> +
>>  #define QERR_INVALID_PASSWORD \
>>      "{ 'class': 'InvalidPassword', 'data': {} }"
>>
Luiz Capitulino March 22, 2010, 12:34 a.m. UTC | #3 
On Fri, 19 Mar 2010 22:05:53 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Thu, 18 Mar 2010 17:33:11 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  qerror.c |    4 ++++
> >>  qerror.h |    3 +++
> >>  2 files changed, 7 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/qerror.c b/qerror.c
> >> index 9fb817e..97e8d4a 100644
> >> --- a/qerror.c
> >> +++ b/qerror.c
> >> @@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
> >>          .desc      = "Invalid parameter type, expected: %(expected)",
> >>      },
> >>      {
> >> +        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
> >> +        .desc      = "Parameter '%(name)' expects %(expected)",
> >> +    },
> >
> >  Would we need this error if we improve QERR_INVALID_PARAMETER to
> > accept an optional 'expects' parameter?
> 
> QERR_INVALID_PARAMETER_VALUE means "this value can't be accepted for
> this parameter".
> 
> QERR_INVALID_PARAMETER means "this parameter can't be accepted,
> regardless of value".  Pedantically speaking, that's a different error.
> Pragmatically speaking, I doubt clients care much (but I'm the one who
> doubts clients care much for differentiating errors in general, so I'm
> biased).  Practically speaking, we can use a single error class
> 'InvalidParameter' if we don't care for the difference, but I figure we
> still need separate QERR_ definitions, because we want different
> human-readable error messages.
> 
> Want me to do anything here now?

 Well, I agree with the human-readable argument. I'm a bit in doubt
if adding new errors is the solution for that, but it's probably
good enough for the immediate term.

> 
> >> +    {
> >>          .error_fmt = QERR_INVALID_PASSWORD,
> >>          .desc      = "Password incorrect",
> >>      },
> >> diff --git a/qerror.h b/qerror.h
> >> index 870cdc3..5625d54 100644
> >> --- a/qerror.h
> >> +++ b/qerror.h
> >> @@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
> >>  #define QERR_INVALID_PARAMETER_TYPE \
> >>      "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
> >>  
> >> +#define QERR_INVALID_PARAMETER_VALUE \
> >> +    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
> >> +
> >>  #define QERR_INVALID_PASSWORD \
> >>      "{ 'class': 'InvalidPassword', 'data': {} }"
> >>
diff mbox

Patch

diff --git a/qerror.c b/qerror.c
index 9fb817e..97e8d4a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -121,6 +121,10 @@  static const QErrorStringTable qerror_table[] = {
         .desc      = "Invalid parameter type, expected: %(expected)",
     },
     {
+        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
+        .desc      = "Parameter '%(name)' expects %(expected)",
+    },
+    {
         .error_fmt = QERR_INVALID_PASSWORD,
         .desc      = "Password incorrect",
     },
diff --git a/qerror.h b/qerror.h
index 870cdc3..5625d54 100644
--- a/qerror.h
+++ b/qerror.h
@@ -106,6 +106,9 @@  QError *qobject_to_qerror(const QObject *obj);
 #define QERR_INVALID_PARAMETER_TYPE \
     "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
 
+#define QERR_INVALID_PARAMETER_VALUE \
+    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
+
 #define QERR_INVALID_PASSWORD \
     "{ 'class': 'InvalidPassword', 'data': {} }"