Hi,
On Thu, Jan 23, 2020 at 2:02 AM Ladislav Michl <
address@hidden> wrote:
>
> On Mon, Jan 20, 2020 at 01:07:35PM +0100, Pawel Kot wrote:
> > On Mon, Jan 20, 2020 at 9:40 AM Ladislav Michl <
address@hidden> wrote:
> > > ... Alternatively I can turn it into proper device plugin architecture.
> > > Is that your preferred way to go? (while doing that I would also add
> > > support for external even loops)
> >
> > Yes, if we're going to change that let's do it in a proper way. Happy to
> > discuss on IRC sometime.
>
> Your IRC connection is unstable beyond being usefull for any discussion ;-)
I'm travelling a lot during a week. Weekends are more peaceful :)
> So here is a quick draft, patch is based on another unreleased one moving
> device_script into separate file. Please note it is only a draft, compile
> tested only (assumes C99 compiler). I tried to avoid device driver changes,
> but patch is still a bit hard to read. Apologies for that. However, it
> should suffice as a beginning of a discussion. I'll polish it a bit more
> based on feedback received and send as separate patch serie.
Looks good. Some minor stuff below.
> +const static gn_device_ops _bluetooth_ops = {
> + .open = _bluetooth_open,
Why just open with _ prefix? Overall question for other places as well.
> + .close = bluetooth_close,
> + .select = bluetooth_select,
> + .read = bluetooth_read,
> + .write = bluetooth_write,
> +};
> gn_error device_nreceived(int *n, struct gn_statemachine *state)
[...]
> + return state->device.ops->nrcvd(state->device.fd, n, state);
I would leave naming consistent like nreceived everywhere. It applies not just here.
> +dnl ======================== Defines location for gettext
> +AC_ARG_WITH(gettext,
> + [ --with-gettext=DIR specifies the base gettext],
> + [ if test x$withval = xyes; then
> + AC_MSG_WARN(Usage is: --with-gettext=DIR)
> + else
> + CFLAGS="$CFLAGS -I$withval"
> + fi
> + ]
> +)
> +
Hm?
> +dnl ======================== Checks for gethostbyname support
> +AC_CHECK_FUNC(gethostbyname, ,
> + AC_CHECK_LIB(nsl, gethostbyname, TCP_LIBS="-lnsl"
> + AC_SUBST(TCP_LIBS)))
> +dnl Haiku requires -lnetwork for socket functions
> +AC_CHECK_FUNC(gethostbyname, ,
> + AC_CHECK_LIB(network, gethostbyname, TCP_LIBS="-lnetwork"
> + AC_SUBST(TCP_LIBS)))
> +
Hm?
> -if test "$enable_phonet" = "yes"; then
> - AC_CHECK_HEADER(linux/phonet.h,
> - [AC_DEFINE(HAVE_SOCKETPHONET, 1, [Whether Phonet is available])
> - USE_SOCKETPHONET="yes"],,
> - [#include <sys/socket.h>
> - #include <linux/phonet.h>])
> -fi
Why?
> -dnl ======================== Checks for gethostbyname support
> -AC_CHECK_FUNC(gethostbyname, ,
> - AC_CHECK_LIB(nsl, gethostbyname, TCP_LIBS="-lnsl"
> - AC_SUBST(TCP_LIBS)))
> -dnl Haiku requires -lnetwork for socket functions
> -AC_CHECK_FUNC(gethostbyname, ,
> - AC_CHECK_LIB(network, gethostbyname, TCP_LIBS="-lnetwork"
> - AC_SUBST(TCP_LIBS)))
> -
OK, so this part is just being moved...
> -if test "$enable_irda" = "yes"; then
> - AC_CHECK_HEADER(linux/irda.h,
> - [AC_DEFINE(HAVE_IRDA, 1, [Whether IrDA is available])
> - USE_IRDA="yes"],,
> - [#include <sys/socket.h>
> - #include <sys/ioctl.h>
> - #include <linux/types.h>])
> -fi
Why?
> -dnl ======================== Defines location for gettext
> -AC_ARG_WITH(gettext,
> - [ --with-gettext=DIR specifies the base gettext],
> - [ if test x$withval = xyes; then
> - AC_MSG_WARN(Usage is: --with-gettext=DIR)
> - else
> - CFLAGS="$CFLAGS -I$withval"
> - fi
> - ]
> -)
OK, so this part is just being moved.
> -void device_reset(struct gn_statemachine *state);
Why?
> typedef struct {
> int fd;
> - gn_connection_type type;
Won't we need it anymore?
> + const gn_device_ops *ops;
> void *device_instance;
> } gn_device;
Cheers,
--
Pawel Kot