Re: [PATCH 3/6] IrDA: IrLAP raw mode

[Ingo Oeser]

Hi Samuel,

Samuel Ortiz wrote:
 This patch allows us to bypass the IrDA stack down to the IrLAP level.
 Sending and receiving frames is done through a character device.
 This is useful for e.g. doing real IrDA sniffing, testing external IrDA
 stacks and for Lirc (once I will add the framing disabling switch).

Nice!
 
 --- /dev/null
 +++ b/include/net/irda/irlap_raw.h
 @@ -0,0 +1,27 @@
 +/*
 + * Copyright (C) 2007 Samuel Ortiz ([EMAIL PROTECTED])
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation, version 2.
 + *
 + */
 +
 +#ifndef _IRLAP_RAW_H
 +#define _IRLAP_RAW_H
 +
 +#ifdef CONFIG_IRDA_RAW
 +
 +int irlap_raw_recv_frame(struct sk_buff *skb, struct net_device *dev);
 +int irlap_raw_register_device(struct net_device * dev);
 +int irlap_raw_unregister_device(struct net_device * dev);
 +
 +#else
 +
 +#define irlap_raw_recv_frame(skbuff, netdev)
 +#define irlap_raw_register_device(netdev)
 +#define irlap_raw_unregister_device(netdev)

This stuff is usually done this way (functions, which just check arguments and 
do nothing):

static inline int irlap_raw_recv_frame(struct sk_buff *skb, struct net_device 
*dev)
{
if (skb == NULL)
return -EINVAL;

if (dev-atalk_ptr == NULL)
return -ENODEV;

return 0;
}

static inline int irlap_raw_register_device(struct net_device * dev)
{
if (dev == NULL)
return -ENODEV;
return 0;
}

statice inline int irlap_raw_unregister_device(struct net_device * dev)
{
if (dev == NULL)
return -ENODEV;
return 0;
}


 --- a/net/irda/irlap_event.c
 +++ b/net/irda/irlap_event.c
 @@ -241,6 +241,11 @@ void irlap_do_event(struct irlap_cb *self, IRLAP_EVENT 
 event,
   if (!self || self-magic != LAP_MAGIC)
   return;
  
 +   if (self-raw_mode)
 +   return;
 +#endif
 +

I would suggest a small helper function here, which compiles into a constant, 
if raw_mode is not compiled in.

like

#ifdef CONFIG_IRDA_RAW
static inline int irlap_raw_mod(struct irlap_cb *self)
{
return self-raw_mode;
}
#else
static inline int irlap_raw_mod(struct irlap_cb *self)
{
return 0;
}
#endif


Best Regards

Ingo Oeser
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] IrDA: IrLAP raw mode

[Samuel Ortiz]

Hi Ingo,

On Fri, Mar 16, 2007 at 11:02:04AM +0100, Ingo Oeser wrote:
  --- /dev/null
  +++ b/include/net/irda/irlap_raw.h
  @@ -0,0 +1,27 @@
  +/*
  + * Copyright (C) 2007 Samuel Ortiz ([EMAIL PROTECTED])
  + *
  + * This program is free software; you can redistribute it and/or
  + * modify it under the terms of the GNU General Public License as
  + * published by the Free Software Foundation, version 2.
  + *
  + */
  +
  +#ifndef _IRLAP_RAW_H
  +#define _IRLAP_RAW_H
  +
  +#ifdef CONFIG_IRDA_RAW
  +
  +int irlap_raw_recv_frame(struct sk_buff *skb, struct net_device *dev);
  +int irlap_raw_register_device(struct net_device * dev);
  +int irlap_raw_unregister_device(struct net_device * dev);
  +
  +#else
  +
  +#define irlap_raw_recv_frame(skbuff, netdev)
  +#define irlap_raw_register_device(netdev)
  +#define irlap_raw_unregister_device(netdev)
 
 This stuff is usually done this way (functions, which just check arguments 
 and do nothing):

Since those are exported symbols, I think it makes sense to do these checks.

 
 I would suggest a small helper function here, which compiles into a constant, 
 if raw_mode is not compiled in.
 
 like
This would prevent us from adding #ifdefs in the core IrDA code: I'll go for
it.

Thanks for the comments.

Cheers,
Samuel.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html