Fix "Dialer: Error setting antenna power" failure at startup

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.

(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)

On 9/28/07, Mike (mwester) <mwesterdls.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.

> (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)

regards
Philipp

Hi,

On 28/09/2007, pHilipp Zabel <philipp.zabelgmail.com> wrote:
> On 9/28/07, Mike (mwester) <mwesterdls.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.

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

On September 28, 2007 , andrzej zaborowski <balrogggmail.com> writes:
> Hi,
>
> On 28/09/2007, pHilipp Zabel <philipp.zabelgmail.com> wrote:
> > On 9/28/07, Mike (mwester) <mwesterdls.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)

Mike (mwester) wrote:
> 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.

IMHO, you should just commit this quick fix now (so that
people can make
phone calls again), and then the official maintainer for
gsmd (who is
that, by the way?) can do the return value interface change
throughout
the whole codebase.

-- Rid