Re: [PATCH] Fix NR_KEYS off-by-one error

OGAWA Hirofumi
2004-07-29T04:11:24Z

Paul Jackson <pj@sgi.com> writes:

> -#define i (tmp.kb_index)
> -#define s (tmp.kb_table)
> -#define v (tmp.kb_value)
> static inline int
> do_kdsk_ioctl(int cmd, struct kbentry __user *user_kbe, int perm, struct kbd_struct *kbd)
> {
> struct kbentry tmp;
> ushort *key_map, val, ov;
> + unsigned char s, i;
> + unsigned short v;
>
> if (copy_from_user(&tmp, user_kbe, sizeof(struct kbentry)))
> return -EFAULT;
>
> + s = tmp.kb_table;
> + i = tmp.kb_index;
> + v = tmp.kb_value;

Yes, s/i/v macro is ugly. But I think it shouldn't, because it limits
the range when struct kbentry was changed. (I think this change is likely)

Perhaps only expand the macro... hmm..

> + if (s >= ARRAY_SIZE(key_maps))
> + return -EINVAL;
> + if (i >= ARRAY_SIZE(key_map))
> + return -EINVAL;

key_map is pointer, so ARRAY_SIZE() can't use. Anyhow these will be
warned by gcc.

Although overhead is insignificance, I'd hated to add overhead of this
test because test is not needed right now.
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>