Commit report for master branch

Brian Candler B.Candler at pobox.com
Fri Mar 22 12:42:47 CET 2013


On Fri, Mar 22, 2013 at 12:00:01AM +0100, The git bot wrote:
> ====== 
> Remove redundant code
> 
> Arran Cudbard-Bell at 2013-03-21T17:41:30Z
> Files modified:
> 	* src/lib/valuepair.c
> 
> Commit diff:
> https://github.com/FreeRADIUS/freeradius-server/commit/89bb28d8be0053202363e191b02f267d6f3cce28
> ====== 

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#ga1ee43e9ef59fc4edfce9a12b9cc54a63
"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));
-               }



More information about the Freeradius-Devel mailing list