5 ms·
> You can have 100% MISRA conformant spaghetti. IMHO some of MISRA rules lead directly to hard to read/maintain code. Consider a function that: 1. opens a fil
by acprog42 8y ago
> You can have 100% MISRA conformant spaghetti.
IMHO some of MISRA rules lead directly to hard to read/maintain code. Consider a function that:
1. opens a file
2. allocates enough memory to store the whole file
3. reads the file
4. return the allocated buffer on success or NULL on failure
You want to write it so it doesn't leak either memory or file handles whether successful or not (if successful ownership of the memory buffer is passed to the caller so it must not be freed in that case).
To be MISRA compliant you'd either end up with a "Christmas tree" of nested scopes or if-statements with extra && in them (pseudo C-code):
char *buf = NULL;
FILE *fh = fopen(...);
if (fh) {
if (success(fseek(fh, end))) {
long int sz = ftell(fh);
if (sz > 0) {
buf = malloc(sz);
if (buf) {
if (failed(fread(fh, buf))) {
report_error();
free(buf);
buf = NULL;
}
} else {
report_error();
}
} else {
report_error();
}
} else {
report_error();
}
fclose(fh);
} else {
report_error();
}
return buf;
However if you were allowed to use goto with a single exit-label the code would be much cleaner and easier to follow:
char *buf = NULL;
char *rv = NULL;
FILE *fh = fopen(...);
if (!fh) {
report_error();
goto exit;
}
if (fail(fseek(fh, end))) {
report_error();
goto exit;
}
long int sz = ftell(fh);
if (sz <= 0) {
report_error();
goto exit;
}
buf = malloc(sz);
if (!buf) {
report_error();
goto exit;
}
if (failed(fread(fh, buf))) {
report_error();
goto exit;
}
rv = buf;
buf = NULL;
exit:
if (fh) {
fclose(fh);
}
if (buf) {
free(buf);
}
return rv;
- acprog42 8y agoToo little karma to edit and fix indentation. Anyway given the above functions, think about adding support for only returning the buffer if the file contains a specific word. While certainly doable in both cases I would at least feel more uncertain editing the first without accidentally creating a bug...
- regularfry 8y agoIf you allowed yourself a second label, you could clean up those repeated `report_error()` calls, too.
- acprog42 8y agoTrue, it would be kind of analogue to try/except/finally if you do that.
- Jedi72 8y agoI'm so glad I don't use C hahaha. Y'all are crazy!
- carlmr 8y agoI completely agree. Most Misra software is embedded without dynamic allocation though, so you usually don't get into this situation too often. I think an exception here makes sense
- acprog42 8y ago> Most Misra software is embedded without dynamic allocation though True, but I've seen enough MISRA code bases plagued with the "Christmas tree" layout even if dynamic memory allocation isn't allowed. This was just a generic example most people can relate to.
- carlmr 8y agoI just checked, and MISRA 2012 seems to allow goto again under certain preconditions (you have to have a label declared in the same function and it has to be in the same block). So actually you can do proper error handling again. Still single return statement, which usually makes for more spaghetti.
- acprog42 8y agoYeah, seems like it actually allows my second example which is good! :)
- kelvich 8y agoActually you can do that without goto's, by using do {} while loop and break: FILE *fh; char *buf = NULL; char *rv = NULL; do { fh = fopen(...); if (!fh) { report_error(); break; } if (fail(fseek(fh, end))) { report_error(); break; } long int sz = ftell(fh); if (sz <= 0) { report_error(); break; } buf = malloc(sz); if (!buf) { report_error(); break; } if (failed(fread(fh, buf))) { report_error(); break; } rv = buf; buf = NULL; } while(false); if (fh) fclose(fh); if (buf) free(buf); return rv;
- acprog42 8y agoSure, that would pass the `grep -r goto ` test, but is just a more convoluted way of doing the same thing.
- coldtea 8y agoThat's fine: the intention was not to do something else, but to avoid goto while still doing the same functionality. Plus, it might be slightly more convoluted than goto, but is less convoluted than the original over-nested code, which was the intention.
- acprog42 8y agoWhich brings us back full circle to the point I was originally trying to convey: if a coding standard forces you to write more convoluted and harder-to-read code just to work around some of its rules, that is a clear failure of said standard. Fortunately MISRA seems to have gotten back some sanity in the 2012 revision compared to the one the above JPL standard is based on.
- coldtea 8y ago>Which brings us back full circle to the point I was originally trying to convey: if a coding standard forces you to write more convoluted and harder-to-read code just to work around some of its rules, that is a clear failure of said standard. Only if the "more convoluted and harder-to-read code" is worse than what the standard tries to avoid. A standard that calls for not allocating memory dynamically for example might result in less elegant code than one that does, for example, but that's not a "clear failure" since that away it avoids the uncertainty and variable runtime performance that malloc brings. I'm addressing the general claim here, that "if a coding standard forces you to write more convoluted and harder-to-read code just to work around some of its rules, that is a clear failure of said standard" -- not specifically where the particular no-goto rule was justified. A standard can be perfectly valid and good in restricting things even if that forces programmers to write "less elegant code" -- as long as this is necessary to fulfil some other objective of the standard (e.g. easy formal verification or real-time behavior).
- philpem 8y agoThis is actually one of the specific variations to the "don't use GOTO" rule in MISRA-C-2012. You're allowed to use GOTO provided you're jumping forward, and only for error handling.