Re: [RFC 15/16] android: Implement basic HAL server

[Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com>]

Hi Anderson,

On Fri, Sep 27, 2013 at 01:50:22PM -0400, Anderson Lizardo wrote:
> Hi Andrei,
> 
> On Fri, Sep 27, 2013 at 10:12 AM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > +static gboolean io_session_event(GIOChannel *chan, GIOCondition cond,
> > +                                                               gpointer data)
> > +{
> > +       struct hal_msg_hdr hdr;
> > +       struct hal_msg_rsp rsp;
> > +       uint8_t *buf;
> > +       int sock, len, size;
> > +       uint8_t status;
> > +
> > +       if (cond & G_IO_NVAL)
> > +               return FALSE;
> > +
> > +       sock = g_io_channel_unix_get_fd(chan);
> > +
> > +       if (cond & (G_IO_HUP | G_IO_ERR)) {
> > +               error("%s: error condition %d", __func__, cond);
> > +               /* TODO: handle */
> > +               return FALSE;
> > +       }
> > +
> > +       len = recv(sock, &hdr, sizeof(hdr), MSG_PEEK);
> > +       if (len != sizeof(hdr)) {
> > +               /* TODO: handle */
> > +               return FALSE;
> > +       }
> > +
> > +       size = sizeof(hdr) + hdr.len;
> > +       buf = malloc(size);
> > +       if (!buf)
> > +               return TRUE;
> > +
> > +       len = recv(sock, buf, size, 0);
> > +       if (len != size) {
> > +               /* TODO: handle */
> > +               free(buf);
> > +               return FALSE;
> > +       }
> > +
> > +       status = process_hal_msg(buf, len);
> > +
> > +       free(buf);
> > +
> > +       memcpy(&rsp, &hdr, sizeof(hdr));
> > +       rsp.hdr.len = sizeof(uint8_t);
> > +       rsp.status = status;
> 
> You are not using "rsp" on this function (and not TODO comment explaining why).

will fix this.

> 
> > +       /* Since daemon would be run also on host we have to grant perms */
> > +       chmod(sock_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> > +                                                               S_IWOTH);
> 
> If it is really necessary to have a world writable socket, better
> explain in detail on the comment.

the problem here is that we need to run this on a host and on Android
where HAL is running under user bluetooth, this has to be changed to use
ifdef logic.

Best regards 
Andrei Emeltchenko