GNOME Bugzilla – Bug 729333
orc: Add some checks on the number of variables per type
Last modified: 2014-06-06 11:42:17 UTC
Created attachment 275547 [details] [review] Add some checks on the number of variables per type Up for comments to see if anyone has opinions on the way these checks are done. In particular, adding a new ORC_COMPILE_RESULT_LIMITS in the 0x200 series would seem better, but this would need adding a field to OrcProgram, and there are comments about ABI in there that makes me wary of adding one (there are plenty of unused fields though, which I could union...).
Review of attachment 275547 [details] [review]: ::: orc/orccompiler.c @@ +194,3 @@ ORC_INFO("initializing compiler for program \"%s\"", program->name); + error_msg = orc_program_get_error (program); + if (error_msg && strcmp (error_msg, "")) { It should never be "", right? ::: orc/orcprogram.c @@ +1049,3 @@ + if (program->error_msg) { + free (program->error_msg); + program->error_msg = NULL; Not sure this makes sense, you ideally always want to see the first error... not the last
orc_program_get_error returns "" when no error. Fair point about first error, I'll change that.
Created attachment 275552 [details] [review] add some checks on the number of variables per type
Review of attachment 275552 [details] [review]: Shouldn't it return NULL instead of ""? ::: orc/orcprogram.c @@ +1049,3 @@ + if (!program->error_msg && error) { + program->error_msg = strdup (error); + } Or maybe we can ensure that we never ever get multiple errors? Or that we accumulate and output them all?
It could return NULL. It did seem odd, but I guess it was done for easy printf use. Ensuring no multiple errors seems very hard given some of the code. A lot of it logs an error but does not return (just sets an error and goes on). Accumulating... could make sense in theory, but I'm not sure it's worth it in practice ?
Created attachment 278012 [details] [review] add some checks on the number of variables per type Make error messages more explicit.
commit 988914384acdf30eab22226d79de3541e7e6f02d Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Thu May 1 15:47:41 2014 +0100 Add some checks on the number of variables per type We want to ensure no more than, say, 8 constants are added to a program. Adding more will violate pervasive assumptions in the code, and may lead to various buffer overflows. By trapping these add creation time, we prevent these issues without cluttering the code with range checks. The user is assumed non malicious here. Add a test to check we can add up to and including the limit for a type, but no more.