hoist TLV checks to before creating the VP

because there's no point in creating the VP and then freeing it.

diff --git a/src/bin/radsnmp.c b/src/bin/radsnmp.c
index 13ad1b446942b1856d9109db806d14f79a18788f..c9358de3fc7bb707376759acf1e59743dbbe318c 100644 (file)
--- a/src/bin/radsnmp.c
+++ b/src/bin/radsnmp.c
@@ -309,6 +309,16 @@ static ssize_t radsnmp_pair_from_oid(TALLOC_CTX *ctx, radsnmp_conf_t *conf, fr_d
                da = parent;
        }
 
+       /*
+        *      TLVs are special, they cannot hold values.  The caller should know this!
+        */
+       if (da->type == FR_TYPE_TLV) {
+               if (!value) return slen;
+
+               fr_strerror_const("TLVs cannot hold values");
+               return -(slen);
+       }
+
        MEM(vp = fr_pair_afrom_da(ctx, da));
 
        /*
@@ -316,17 +326,6 @@ static ssize_t radsnmp_pair_from_oid(TALLOC_CTX *ctx, radsnmp_conf_t *conf, fr_d
         */
        if (!value) {
                switch (da->type) {
-               /*
-                *      We can blame the authors of RFC 6929 for
-                *      this hack.  Apparently the presence or absence
-                *      of an attribute isn't considered a useful means
-                *      of conveying information, so empty TLVs are
-                *      disallowed.
-                */
-               case FR_TYPE_TLV:
-                       fr_pair_to_unknown(vp);
-                       FALL_THROUGH;
-
                case FR_TYPE_OCTETS:
                        fr_pair_value_memdup(vp, (uint8_t const *)"\0", 1, true);
                        break;
@@ -346,11 +345,6 @@ static ssize_t radsnmp_pair_from_oid(TALLOC_CTX *ctx, radsnmp_conf_t *conf, fr_d
                return slen;
        }
 
-       if (da->type == FR_TYPE_TLV) {
-               fr_strerror_const("TLVs cannot hold values");
-               return -(slen);
-       }
-
        ret = fr_pair_value_from_str(vp, value, strlen(value), NULL, true);
        if (ret < 0) {
                slen = -(slen);