Messages in this thread | | | Date | Sat, 10 Nov 2007 18:05:57 +0100 | From | Jakob Oestergaard <> | Subject | Re: [PATCH] Smackv10: Smack rules grammar + their stateful parser(2) |
| |
... > I've double-checked the code for any possible off-by-one/overflow > errors. ...
Two things caught my eye.
... > + case bol: > + case subject: > + if (*label_len >= SMK_MAXLEN) > + goto out; > + subjectstr[(*label_len)++] = data[i];
Why is the '>' necessary? Could it happen that you had incremented past the point of equality?
If that could not happen, then in my oppinion '>=' is very misleading when '==' is really what is needed.
... > + case object: > + if (*prevstate == blank) { > + subjectstr[*label_len] = '\0'; > + *label_len = 0; > + }
I wonder why it is valid to uncritically use the already incremented label_len here, without checking its value (like is done above).
It seems strangely asymmetrical. I'm not saying it's wrong, because there may be a subtle reason as to why it's not, but if that's the case then I think that subtle reason should be documented with a comment.
... > + case access: > + if (*prevstate == blank) { > + objectstr[*label_len] = '\0'; > + *label_len = 0; > + }
Same applies here.
--
/ jakob
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |