[2/6] QError: New QERR_FOPEN_FAILED

Submitted by Markus Armbruster on Jan. 15, 2010, 4:25 p.m.

Details

Message ID 1263572729-14813-3-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

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

Comments

Luiz Capitulino Jan. 18, 2010, 2:23 p.m.
On Fri, 15 Jan 2010 17:25:25 +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 5f8fc5d..e7b8ca7 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -73,6 +73,10 @@ static const QErrorStringTable qerror_table[] = {
>          .desc      = "No file descriptor supplied via SCM_RIGHTS",
>      },
>      {
> +        .error_fmt = QERR_FOPEN_FAILED,
> +        .desc      = "Could not open '%{filename}'",
> +    },

 Shouldn't this be something like QERR_OPEN_FAILED, so that we
can use the same error for all open functions?

 Also, we have to think a way to specify the reason from errno.
Luiz Capitulino Jan. 18, 2010, 2:38 p.m.
On Mon, 18 Jan 2010 12:23:28 -0200
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Fri, 15 Jan 2010 17:25:25 +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 5f8fc5d..e7b8ca7 100644
> > --- a/qerror.c
> > +++ b/qerror.c
> > @@ -73,6 +73,10 @@ static const QErrorStringTable qerror_table[] = {
> >          .desc      = "No file descriptor supplied via SCM_RIGHTS",
> >      },
> >      {
> > +        .error_fmt = QERR_FOPEN_FAILED,
> > +        .desc      = "Could not open '%{filename}'",
> > +    },
> 
>  Shouldn't this be something like QERR_OPEN_FAILED, so that we
> can use the same error for all open functions?

 Something I forgot to mention, 'FopenFailed' doesn't seem
interesting to clients. Maybe 'OpenFileFailed'?
Markus Armbruster Jan. 18, 2010, 2:52 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Fri, 15 Jan 2010 17:25:25 +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 5f8fc5d..e7b8ca7 100644
>> --- a/qerror.c
>> +++ b/qerror.c
>> @@ -73,6 +73,10 @@ static const QErrorStringTable qerror_table[] = {
>>          .desc      = "No file descriptor supplied via SCM_RIGHTS",
>>      },
>>      {
>> +        .error_fmt = QERR_FOPEN_FAILED,
>> +        .desc      = "Could not open '%{filename}'",
>> +    },
>
>  Shouldn't this be something like QERR_OPEN_FAILED, so that we
> can use the same error for all open functions?

Whatever name you like best.  The intention is certainly to use this for
*file* open errors regardless of the precise function used to open.

>  Also, we have to think a way to specify the reason from errno.

Yes only if client programs use this for handling the error, not just to
pass it to a human user.

Although most errors are standardized, their numeric encoding is not, so
we can't just transmit errno.  strerror() is right out, because that's
for humans.
Luiz Capitulino Jan. 18, 2010, 4:49 p.m.
On Mon, 18 Jan 2010 15:52:13 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Fri, 15 Jan 2010 17:25:25 +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 5f8fc5d..e7b8ca7 100644
> >> --- a/qerror.c
> >> +++ b/qerror.c
> >> @@ -73,6 +73,10 @@ static const QErrorStringTable qerror_table[] = {
> >>          .desc      = "No file descriptor supplied via SCM_RIGHTS",
> >>      },
> >>      {
> >> +        .error_fmt = QERR_FOPEN_FAILED,
> >> +        .desc      = "Could not open '%{filename}'",
> >> +    },
> >
> >  Shouldn't this be something like QERR_OPEN_FAILED, so that we
> > can use the same error for all open functions?
> 
> Whatever name you like best.  The intention is certainly to use this for
> *file* open errors regardless of the precise function used to open.

 Probably OpenFileFailed or CantOpenFile.

> >  Also, we have to think a way to specify the reason from errno.
> 
> Yes only if client programs use this for handling the error, not just to
> pass it to a human user.
> 
> Although most errors are standardized, their numeric encoding is not, so
> we can't just transmit errno.  strerror() is right out, because that's
> for humans.

 IIRC we had a conversation about this and the conclusion was that we
will need our own qerror_strerror() or something like that.

Patch hide | download patch | download mbox

diff --git a/qerror.c b/qerror.c
index 5f8fc5d..e7b8ca7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -73,6 +73,10 @@  static const QErrorStringTable qerror_table[] = {
         .desc      = "No file descriptor supplied via SCM_RIGHTS",
     },
     {
+        .error_fmt = QERR_FOPEN_FAILED,
+        .desc      = "Could not open '%{filename}'",
+    },
+    {
         .error_fmt = QERR_INVALID_BLOCK_FORMAT,
         .desc      = "Invalid block format %(name)",
     },
diff --git a/qerror.h b/qerror.h
index 9e220d6..8701570 100644
--- a/qerror.h
+++ b/qerror.h
@@ -64,6 +64,9 @@  QError *qobject_to_qerror(const QObject *obj);
 #define QERR_FD_NOT_SUPPLIED \
     "{ 'class': 'FdNotSupplied', 'data': {} }"
 
+#define QERR_FOPEN_FAILED \
+    "{ 'class': 'FopenFailed', 'data': { 'filename': %s } }"
+
 #define QERR_INVALID_BLOCK_FORMAT \
     "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"