assuan_read_from_server() and return value

[Marcus Brinkmann]

At Sat, 15 Dec 2007 12:32:00 -0500,
Ben Kibbey <bjk at luxsci.net> wrote:
> 
> On Wed, Dec 12, 2007 at 03:47:18PM +0100, Marcus Brinkmann wrote:
> > > After the inquire callback is finished, assuan_transact() sends END to
> > > terminate the inquire then calls _assuan_read_from_server() again.
> > 
> > Are you sure?  That's not my reading of the code:
> > 
> > assuan-client.c::assuan_transact()
> > [...]
> > 
> >       if (!inquire_cb)
> >         {
> >           assuan_write_line (ctx, "END"); /* get out of inquire mode */
> >           _assuan_read_from_server (ctx, &okay, &off); /* dummy read */
> >           rc = _assuan_error (ASSUAN_No_Inquire_Callback);
> >         }
> >       else
> >         {
> >           rc = inquire_cb (inquire_cb_arg, line);
> >           if (!rc)
> >             rc = assuan_send_data (ctx, NULL, 0); /* flush and send END */
> >           if (!rc)
> >             goto again;
> >         }
> 
> I do have an inquire callback set. I was referring to the END sent from
> assuan_send_data().

Ok.

> Then _assuan_read_from_server() is called to get the
> error code (if any) of the inquire (with my patch applied).

Ah, so now we are talking about the _assuan_read_from_server at the
"again:" label in _assuan_transact?

In this case, the ERR should set OKAY to 0 and OFF to the offset of
the error code, and then assuan_transact should execute:

  if (!okay)
    {
      rc = atoi (line);
      if (rc > 0 && rc < 100)
        rc = _assuan_error (ASSUAN_Server_Fault);
      else if (rc > 0 && rc <= 405)
        rc = _assuan_error (rc);
    }

Thereby returning the error code as it should.  Why is that code not
working for you?

> > > Without the patch, _assuan_read_from_server()
> > > returns the value returned from _assuan_read_line() (zero). I use
> > > GPG_ERR codes on the server. Pinentry uses ASSUAN_ errors and there is
> > > no way to tell if the error is an GPG_ERR_ or an ASSUAN_ error.
> > 
> > Not quite correct.  We have a heuristic to differentiate: Values that
> > have the high bits set indicate a gpg error with a source set.  All
> > our servers set a gpg error source.  On the other hand, low values
> > indicate an assuan based error code.  This is not failure proof, but
> > covers current and expected usage.  By the way: It is currently
> > undefined how assuan error codes can be extended by user defined error
> > codes.  In gpg-error, there are reserved values for that.
> 
> Breaks when GPG_ERR_SOURCE_UNKNOWN is used, and this is the default.

Yes, but sometimes the right response to "this hurts" is "don't do it
then".  As there is no good reason not to set the error source, it
should be set if gpg-error is used.

> > > So I
> > > guess pinentry has to migrate to use GPG_ERR_ codes, which may break
> > > clients, or I have to wait until libassuan removes ASSUAN_ codes
> > > completely forcing libassuan users to use GPG_ERR_ codes, which seems
> > > unlikely to happen any time soon?
> > 
> > We can map assuan error codes to gpg error codes (see the function
> > _assuan_error in mkerrors).
> 
> Has the error source problem above.
> 
> I have rewritten my app to handle pinentry specially, although it will
> break when pinentry uses gpg-error codes.

It shouldn't break if pinentry sets an error source, which it would if
we modify it to use gpg-error.

> But I still need the patch
> applied in order to get any inquire error code from the server.

I am still (by now probably even more than in the beginning) as to
what the actual error case code flow is.  At the risk of trying your
patience here, can you walk me through it one more time?

Thanks,
Marcus