On Fri, Mar 22, 2013 at 12:00:01AM +0100, The git bot wrote:
====== Remove redundant code
Arran Cudbard-Bell@2013-03-21T17:41:30Z Files modified: * src/lib/valuepair.c
Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/89bb28d8be0053202363e... ======
Observation: the code now does #define VERIFY(_x) _x=talloc_get_type(_x, VALUE_PAIR) and my understanding is that if the type is wrong, _x will be set to null. http://talloc.samba.org/talloc/doc/html/group__talloc.html#ga1ee43e9ef59fc4e... "Returns: The properly casted pointer given by ptr, NULL on error." However the code then goes ahead and indirects through vp without first checking if vp is null, and there are lots of instances of this. Is that intentional? I guess if all you want is a segfault on type error then it's OK (but in that case I don't see why you don't use talloc_get_type_abort) Regards, Brian. --- a/src/lib/valuepair.c +++ b/src/lib/valuepair.c @@ -41,7 +41,7 @@ RCSID("$Id$") /* * Requires typeof(), which is in most modern C compilers. */ -#define VERIFY(_x) _x=talloc_get_type_abort(_x, VALUE_PAIR) +#define VERIFY(_x) _x=talloc_get_type(_x, VALUE_PAIR) #else #define VERIFY(_x) #endif @@ -122,18 +122,10 @@ void pairbasicfree(VALUE_PAIR *vp) VERIFY(vp); /* - * The lack of DA probably means something has gone - * wrong, try and get as much info as we can before - * calling talloc_free (at which point we'll probably get - * an abort). + * The lack of DA means something has gone wrong */ if (!vp->da) { fr_strerror_printf("VALUE_PAIR has NULL DICT_ATTR pointer (probab - - if (!talloc_get_type(vp, VALUE_PAIR)) { - fr_strerror_printf("Pointer being freed is NOT a VALUE_PA - talloc_get_name(vp)); - }