Hi,

> +/* wrappers of WE functions that don't check for errors */
> +static inline int _iwe_stream_add_point(struct translate_scan *data,
> 
>       You may wonder why the original API of iwe_stream_add_point(()
> is the way it is rather than the way you expect. I'll explain :
> working closely with people doing embedded systems, one of my prime
> concern has always been footprint and code simplicity, and this
> explain those choices.

...

> 
>       Obviously, in your case it would be slightly different, so you
> could do something like this :
> ------------------------------------------------------
>       res = ieee80211_networks_foreach(ieee, ieee80211_translate_scan, &data);
>       if (res > 0) res = 0;
>       if (data.stop - data.start <= IW_EV_ADDR_LEN)
>               res = -E2BIG;
>       wrqu->data.length = data.start - extra;
> ------------------------------------------------------
>       Then, obviously, you would be able to drop all the checks in
> _iwe_stream_add_event(). Would you mind to try that and see how it
> affect footprint and code readability ?

You're suggesting this check:
        if (data.stop - data.start <= IW_EV_ADDR_LEN)

Think of this sequence: add(8b); add(10b); add(8b) -- adding the
ten bytes could silently do nothing, but subsequently adding
eight bytes might succeed

This would only work in cases where the largest event put into
the stream would not exceed IW_EV_ADDR_LEN bytes; This is not the
case. Think of the rates that are passed as a freeform string
in an IWEVCUSTOM event (which is bad anyway).

Even if we used the size of the largest possible event in the
check above, I don't like the idea of lying to the userspace -
returning -E2BIG even if there is enough space in the buffer.
Let alone how error-prone the resulting code would be - anyone
adding another event to the scan results would need
to understand this dirty hack...

I think the wrapper really makes the code cleaner and more
readable.

>       This brings us to my second point. You don't seem to support
> arbitrary size scan buffer (WE-17). That would be a trivial
> modification of your code, and allow to remove the arbitrary limit on
> IW_SCAN_MAX_DATA.

Good point, thanks. Will be fixed.


Regards,

-- 
Jirka Bohac <[EMAIL PROTECTED]>
SUSE Labs, SUSE CR

-
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

Reply via email to