[PATCH] Fix "Dialer: Error setting antenna power" failure at startup

[Mike (mwester)]

On September 28, 2007 , andrzej zaborowski <balrogg at gmail.com> writes:
> Hi,
>
> On 28/09/2007, pHilipp Zabel <philipp.zabel at gmail.com> wrote:
> > On 9/28/07, Mike (mwester) <mwester at dls.net> wrote:
> > > Current images are crippled because the dailer fails at startup with
the
> > > message "Dialer: Error setting antenna power".
> > >
> > > This problem is traced back to a recent change committed to OE that
adds
> > > error-checking logic to the read/write calls in libgsmd.  Basically,
> > > lgsm_send_simple() calls lgsm_send(), and expects the return value to
be the
> > > number of characters sent.  The recent patch to add error-checking
changed
> > > lgsm_send() to return a 0 on success, which results in
lgsm_send_simple()
> > > always returning the error code EIO on success.
> > >
> > > The patch below restores the old behavior for lgsm_send(), such that
it
> > > returns the number of characters on succuss.  Arguably it might be
better to
> > > change the callers behavior such that they all expect the
more-standard "0"
> > > return value for success.
> >
> > Comparing with posix send(), I think returning the number of bytes
> > sent is the most consistent thing to do.
>
> Yes, on one hand this is consistent with the standard send() but on
> the other the caller shouldn't be concerned with the number of bytes
> sent now that lgsm_send returns only after all bytes have been sent or
> an error occured.
>
> And I should apologise for the confusion, it seems it was me who
> caused the breakage.
>
> >
> > > (Unless anyone has any strong objections in the next day, I'll commit
this
> > > simple fix to OE - at least as an interim solution so that we can get
> > > current images to function correctly again)
> > >
> > > Thanks!
> > > Mike (mwester)
> > >
> > > --- gsm/src/libgsmd/libgsmd.c.orig      2007-09-25
00:41:56.000000000 -0500
> > > +++ gsm/src/libgsmd/libgsmd.c   2007-09-25 00:43:44.000000000 -0500
> > >  -210,7 +210,7 @@
> > >                         pos += rc;
> > >                 }
> > >         }
> > > -       return 0;
> > > +       return (sizeof(*gmh) + gmh->len);
> > >  }
> > >
> > >  struct gsmd_msg_hdr *lgsm_gmh_fill(int type, int subtype, int
payload_len)
>
> Here's the change that I would make in the original patch if I had
> noticed that lgsm_send was being used in lgsm_send_simple, but I'm not
> insisting on applying this one.

This resolves the issue for lgsm_send_simple(), but the code in the library
is sprinkled with other calls to lgsm_send() that also make the assumption
that it returns the number of bytes sent.  I guess the fact that the dialer
is the only app that has actually put up a dialog box announcing a failure
means that we don't have a whole lot of clients using the library -- and
perhaps the ones that do aren't doing a good job of checking return codes :(

Changing the other places where lgsm_send() is called to do the comparison
to zero would probably simplify the code, but the big reason I didn't tackle
this for the "quick fix" was that in lgsm_pin(), the return value from
lgsm_send() is passed back to the client using the library.  Hence, changing
lgsm_send()'s semantics would have the side-effect of changing the libgsmd
API, and that's not something I felt a "quick fix" should undertake.

(That said, I did find the client that used lgsm_pin(), and it doesn't check
the return code at all currently.)

At this point, I think I still would favor checking in the quick fix so that
the images work again.  But it does seem that more discussion might be a
good thing, especially considering that the return code from lgsm_send()
"leaks through" to the clients that call the library.


> diff --git a/src/libgsmd/libgsmd.c b/src/libgsmd/libgsmd.c
> index cc804ed..ee8bd81 100644
> --- a/src/libgsmd/libgsmd.c
> +++ b/src/libgsmd/libgsmd.c
> @@ -238,11 +238,7 @@ int lgsm_send_simple(struct lgsm_handle *lh, int
type, int
> sub_type)
>         if (!gmh)
>                 return -ENOMEM;
>         rc = lgsm_send(lh, gmh);
> -       if (rc < gmh->len + sizeof(*gmh)) {
> -               lgsm_gmh_free(gmh);
> -               return -EIO;
> -       }
> -       lgsm_gmh_free(gmh);
>
> -       return 0;
> +       lgsm_gmh_free(gmh);
> +       return rc;
>  }
>
> Regards
>

Thanks,
Mike (mwester)