[cvs] bogofilter/src bogomain.c, 1.10, 1.11 datastore_db_trans.c, 1.18, 1.19 datastore_sqlite.c, 1.17, 1.18 datastore_tdb.c, 1.45, 1.46

[Matthias Andree]

David Relson <relson at users.sourceforge.net> writes:

> Modified Files:
> 	bogomain.c datastore_db_trans.c datastore_sqlite.c 
> 	datastore_tdb.c 
> Log Message:
> Additionall free/xfree cleanup.

Spraying xfree() everywhere free() is with a watering can isn't a good
idea. free() is appropriate when we know the memory is definitely
allocated, for instance, after malloc(), and perhaps in code that is
executed only once during the lifetime of a program run.

Particularly in places like the one shown below, where we're already
very expensive and in sort-of-inner-loops, replacing faster free() calls
(often macros) with our "expensive" xfree() function isn't exactly the
best idea and gives rise to performance concerns.

There are times when it's useful to have slightly different semantics,
apart from the POSIX requirement

    "The free() function shall cause the space pointed to by ptr to be
     deallocated; that is, made available for further allocation. If ptr
     is a null pointer, no action shall occur. (...)"
                    (IEEE Std 1003.1-2001, free - free allocated memory)

that makes xfree() a gratuitious addition that should be folded down to
free() on all systems where free(NULL) is safe.

I'll review which of these are safe to use free().

>  /** Executes the SQL statement \a cmd on the database \a db and returns
> @@ -167,8 +167,8 @@
>  		    if (key.leng != strlen(ENDIAN32)
>  			    || memcmp(key.data, ENDIAN32, key.leng) != 0)
>  			rc = hook(&key, &val, userdata);
> -		    free(val.data);
> -		    free(key.data);
> +		    xfree(val.data);
> +		    xfree(key.data);
>  		    if (rc) {
>  			sqlite3_finalize(stmt);
>  			return rc;
> @@ -188,7 +188,7 @@
>  		return rc;
>  	}
>      }
> -    /* free resources */
> +    /* xfree resources */
>      sqlite3_finalize(stmt);
>      return found ? 0 : DS_NOTFOUND;
>  }
> @@ -244,7 +244,7 @@
>  		    v.leng = sizeof(p);
>  		    if (sqlexec(dbh->db, LAYOUT)) goto barf;
>  		    rc2 = db_set_dbvalue(dbh, &k, &v);
> -		    free(k.data);
> +		    xfree(k.data);
>  		    if (rc2)
>  			goto barf;
>  		    if (sqlexec(dbh->db, "COMMIT;")) goto barf;
> @@ -264,13 +264,13 @@
>  	k.leng = strlen(k.data);
>  
>  	ee = db_get_dbvalue(dbh, &k, &v);
> -	free(k.data);
> +	xfree(k.data);
>  	switch(ee) {
>  	    case 0: /* found endian marker token, read it */
>  		if (v.leng < 4)
>  		    goto barf;
>  		t = ((u_int32_t *)v.data)[0];
> -		free(v.data);
> +		xfree(v.data);
>  		switch (t) {
>  		    case 0x01020304: /* same endian, "UNIX" */
>  			dbh->swapped = false;

-- 
Matthias Andree
_______________________________________________
Bogofilter-dev mailing list
Bogofilter-dev at bogofilter.org
http://www.bogofilter.org/mailman/listinfo/bogofilter-dev