Permalink
Browse files

Defensive programming leads to sloppy programming

Given the functions changed:

	gnugol_init_QueryOptions()
	gnugol_reset_QueryOptions()
	gnugol_free_QueryOptions()

when will you *ever* pass a NULL QueryOptions_t to it?  You won't, unless it's
a programming mistake, but in checking for NULL and doing stuff if it isn't
hides bugs.  One tenent of programming I do:

	THOU SHALT GIVE VALID PARAMETERS TO A FUNCTION
	(and in return, they'll return valid data)

So, with that in mind, get rid of the unnecessary checks for NULL (remember:
you won't be passing in NULL, now will you?)  Also, free() accepts a NULL
(it won't do anything) so there's no need to check for that condition.  And
besides, those pointers won't be NULL anyway, right?  Right.

If this gives you the heebie-jeebies, you haven't programmed enough with this
style.  It really does make for rock solid code.  But you do need to learn
to live with assert().
  • Loading branch information...
1 parent 2287614 commit d8b058fcf3d3bc3c835ac239652d34c74829667c @spc476 spc476 committed Jan 9, 2011
Showing with 46 additions and 32 deletions.
  1. +46 −32 src/common/format.c
View
@@ -4,6 +4,7 @@
#include <fcntl.h>
#include <errno.h>
#include <string.h>
+#include <assert.h>
#include "query.h"
#include "formats.h"
#include "utf8.h"
@@ -28,47 +29,60 @@ meta_charset_map = {
*/
+int gnugol_init_QueryOptions(QueryOptions_t *q)
+{
+ assert(q != NULL);
+
+ memset(q,0,sizeof(QueryOptions_t));
+ q->err.size = 4 * 1024;
+ q->out.size = 64 * 1024;
+ q->wrn.size = 4 * 1024;
+
+ q->err.s = malloc(q->err.size);
+ q->out.s = malloc(q->out.size);
+ q->wrn.s = malloc(q->wrn.size);
+
+ return (q->err.s == NULL) || (q->out.s == NULL) || (q->wrn.s == NULL);
+}
-// FIXME: Be more paranoid
+int gnugol_reset_QueryOptions(QueryOptions_t *q)
+{
+ assert(q != NULL); /* when will this *ever* be false? */
+
+ buffer_obj_t terr = q->err;
+ buffer_obj_t twrn = q->wrn;
+ buffer_obj_t tout = q->out;
+ memset(q,0,sizeof(QueryOptions_t));
+ q->err = terr;
+ q->wrn = twrn;
+ q->out = tout;
+ q->err.len = q->out.len = q->wrn.len = 0;
-int gnugol_init_QueryOptions(QueryOptions_t *q) {
- if(q != NULL) {
- memset(q,0,sizeof(QueryOptions_t));
- if((q->err.s = (char *) malloc(4096)) != NULL) q->err.size = 4096;
- if((q->out.s = (char *) malloc(1024*64)) != NULL) q->out.size = (1024*64);
- if((q->wrn.s = (char *) malloc(4096)) != NULL) q->wrn.size = (4096);
- } else {
- return(1);
- }
return(0);
}
-int gnugol_reset_QueryOptions(QueryOptions_t *q) {
- if(q != NULL) {
- buffer_obj_t terr = q->err;
- buffer_obj_t twrn = q->wrn;
- buffer_obj_t tout = q->out;
- memset(q,0,sizeof(QueryOptions_t));
- q->err = terr;
- q->wrn = twrn;
- q->out = tout;
- q->err.len = q->out.len = q->wrn.len = 0;
- } else {
- return(1);
- }
- return(0);
-}
+int gnugol_free_QueryOptions(QueryOptions_t *q)
+{
+ assert(q != NULL);
+
+ free(q->err.s);
+ free(q->out.s);
+ free(q->wrn.s);
+
+#ifndef NDEBUG
+ q->err.s = q->out.s = q->wrn.s = NULL;
+ q->err.size = q->err.len = 0;
+ q->out.size = q->out.len = 0;
+ q->wrn.size = q->wrn.len = 0;
+#endif
-int gnugol_free_QueryOptions(QueryOptions_t *q) {
- if(q != NULL) {
- if(q->err.s != NULL) { free(q->err.s); q->err.len = q->err.size = 0; }
- if(q->out.s != NULL) { free(q->out.s); q->out.len = q->out.size = 0; }
- if(q->wrn.s != NULL) { free(q->wrn.s); q->wrn.len = q->wrn.size = 0; }
- }
return(0);
}
-int gnugol_header_out(QueryOptions_t *q) {
+int gnugol_header_out(QueryOptions_t *q)
+{
+ assert(q != NULL);
+
if(q->header) {
char buffer[SNIPPETSIZE];
strncpy(buffer,q->keywords,SNIPPETSIZE);

0 comments on commit d8b058f

Please sign in to comment.