Hi Marcel.
> > +static GSList *pending_requests;
> > +static GIOChannel *channel;
> > +static guint32 rtnl_seqnr;
> > +static guint rtnl_watch;
>
> To be fair, you don't need both, the GIOChannel and the watch. You can
> just add the watch and then unref the GIOChannel and set it to close on
> unref. That way when the watch gets removed or you remove it, the
> GIOChannel and the underlaying socket will be automatically closed as
> well.
....
> As described above. Just unref the GIOChannel and forget about it. And
> then you can make the GIOChannel a local variable.
I think I have to keep the channel, because I do need the
file descriptor derived from the channel when doing sendto.
> > > You don't really have to play this append/remove game. You can just
> > > append it after send_rtnl_req succeed. The read goes via the
> mainloop
> > > anyway and we are a single threaded program. So the order is
> > > guaranteed.
...
> You don't need to append it first, send the message, check error and in
> case of an error remove it again.
OK, thanks - now I get what your after. I completely misunderstood your
previous comment, my bad.
> And while we are at it. Essentially you also need to use asynchronous
> and non-blocking watch driven sending of your message in the first
> place. I did forget to check if you open the RTNL socket non-blocking,
> but you should be doing that.
So I'll add something like this then:
fl = fcntl(sk, F_GETFL);
fcntl(sk, F_SETFL, fl | O_NONBLOCK);
> > + }
> > +
> > + g_slist_free(pending_requests);
> > + pending_requests = NULL;
>
> No need to set pending_requests to NULL here.
But I will have to move this to the caif_rtnl_init function then,
I think pending_requests needs to be NULL before using it.
> > > I am not really sold on this param stuct thingy btw. Why do you
> need
> > > them? Just proper input params and output params for the callback
> > > handler would do them same. Won't they?
> >
> > Yes, I could do that. But in that case I would prefer to typedef the
> > response function to simplify create function's signature. Otherwise
> > I think the create function's signature gets too complex.
>
> typedef for function callbacks are just fine. I have no problem with
> that at all.
I got rid of the structs and typedef'ed response function - I agree this better.
<snip>
typedef void (*rtnl_create_cb_t) (int result, gpointer user_data,
char *ifname, int ifindex);
extern int caif_rtnl_create_interface(gpointer user_data, int type, int connid,
gboolean loop, rtnl_create_cb_t cb);
Hoping to get the next patch out later today.
Regards,
Sjur
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
