Re: [PATCH 1/2 v2] New netfilter target to trigger LED devices

[Adam Nielsen <a.nielsen@shikadi.net>]

>> +struct xt_led_info {
>> +	char id[26];        /* Unique ID for this trigger in the LED class */
>> +	__u32 delay;        /* Delay until LED is switched off after trigger */
>> +	__u8 always_blink;  /* Blink even if the LED is already on */
>> +
>> +	void *internal_data; /* Kernel data used in the module */
>> +};
> 
> You now have an alignment problem, and need at least
> 	void *internal_data __attribute__((aligned(8)))
> as explained in the PDF. Ultimatively, these shouls be reordered to
> accomodate for gaps (see doc).
> 
> Also, why is it 26?

Ah bummer, I forgot about that.  Is this the best order?  If I'm
understanding correctly this order shouldn't need member alignment.

struct xt_led_info {
	void *internal_data;
	__u32 delay;
	__u8 always_blink;
	char id[26];
};

The id[26] is for a 16 char ID + the 10 digit "netfilter-" prefix.
Actually that's a good point, it needs to be 27 to accomodate the
terminating NULL.  At any rate, I'm not sure of the best way to describe
that, without using #define.  What do you suggest?

   /* strlen("netfilter-") + 16 char user-definable name */
   #define LED_TRIGGER_ID_LENGTH (10 + 16)

   char id[LED_TRIGGER_ID_LENGTH + 1];

In that case it might be worth #defining the 16 char part separately,
so that it can be used in the parameter verification code too.  Let
me know what you think.

Thanks,
Adam.