[master] ef95a08 Implement three different types of attributes and generate get/set code from table

Martin Blix Grydeland martin at varnish-software.com
Tue Feb 23 17:44:46 CET 2016


commit ef95a084055f1c80754cbf0eac38ec79556a8eae
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Fri Oct 30 10:36:42 2015 +0100

    Implement three different types of attributes and generate get/set code from table
    
    Fixed size attributes has it's space always reserved.
    
    Variable size attributes change in size, and their combined total
    length needs to be given to allocobj.
    
    Auxiliary attributes do not need to be predeclared.
    
    The code for setting and getting the attributes is generated from a
    table that lists the different attributes according to their type.
    
    Add documentation about the behaviour of the get/set functions.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index d288bab..d735df2 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -391,9 +391,14 @@ struct boc {
  */
 
 enum obj_attr {
-#define OBJ_ATTR(U, l)  OA_##U,
+#define OBJ_FIXATTR(U, l, s)	OA_##U,
+#define OBJ_VARATTR(U, l)	OA_##U,
+#define OBJ_AUXATTR(U, l)	OA_##U,
 #include "tbl/obj_attr.h"
-#undef OBJ_ATTR
+#undef OBJ_AUXATTR
+#undef OBJ_VARATTR
+#undef OBJ_FIXATTR
+				OA__MAX,
 };
 
 enum obj_flags {
diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c
index 13f814a..57961cf 100644
--- a/bin/varnishd/cache/cache_obj.c
+++ b/bin/varnishd/cache/cache_obj.c
@@ -369,6 +369,10 @@ ObjHasAttr(struct worker *wrk, struct objcore *oc, enum obj_attr attr)
  * ObjGetAttr()
  *
  * Get an attribute of the object.
+ *
+ * Returns NULL on unset or zero length attributes and len set to
+ * zero. Returns Non-NULL otherwise and len is updated with the attributes
+ * length.
  */
 
 const void *
@@ -386,8 +390,23 @@ ObjGetAttr(struct worker *wrk, struct objcore *oc, enum obj_attr attr,
 /*====================================================================
  * ObjSetAttr()
  *
+ * Setting fixed size attributes always succeeds.
+ *
+ * Setting a variable size attribute asserts if the combined size of the
+ * variable attributes exceeds the total variable attribute space set at
+ * object creation. If there is space it always succeeds.
+ *
+ * Setting an auxiliary attribute can fail.
+ *
+ * Resetting any variable asserts if the new length does not match the
+ * previous length exactly.
+ *
  * If ptr is Non-NULL, it points to the new content which is copied into
  * the attribute.  Otherwise the caller will have to do the copying.
+ *
+ * Return value is non-NULL on success and NULL on failure. If ptr was
+ * non-NULL, it is an error to use the returned pointer to set the
+ * attribute data, it is only a success indicator in that case.
  */
 
 void *
diff --git a/bin/varnishd/storage/storage_persistent_silo.c b/bin/varnishd/storage/storage_persistent_silo.c
index 78b7e82..9478702 100644
--- a/bin/varnishd/storage/storage_persistent_silo.c
+++ b/bin/varnishd/storage/storage_persistent_silo.c
@@ -435,7 +435,7 @@ smp_sml_getobj(struct worker *wrk, struct objcore *oc)
 				break;
 			l += st->len;
 		}
-		if (l != vbe64dec(o->oa_len))
+		if (l != vbe64dec(o->fa_len))
 			bad |= 0x100;
 
 		if(bad) {
diff --git a/bin/varnishd/storage/storage_simple.c b/bin/varnishd/storage/storage_simple.c
index f2f2111..8b862db 100644
--- a/bin/varnishd/storage/storage_simple.c
+++ b/bin/varnishd/storage/storage_simple.c
@@ -187,10 +187,14 @@ sml_slim(struct worker *wrk, struct objcore *oc)
 	o = sml_getobj(wrk, oc);
 	CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
 
-	if (o->esidata != NULL) {
-		sml_stv_free(stv, o->esidata);
-		o->esidata = NULL;
+#define OBJ_AUXATTR(U, l)						\
+	if (o->aa_##l != NULL) {					\
+		sml_stv_free(stv, o->aa_##l);				\
+		o->aa_##l = NULL;					\
 	}
+#include "tbl/obj_attr.h"
+#undef OBJ_AUXATTR
+
 	VTAILQ_FOREACH_SAFE(st, &o->list, list, stn) {
 		CHECK_OBJ_NOTNULL(st, STORAGE_MAGIC);
 		VTAILQ_REMOVE(&o->list, st, list);
@@ -489,33 +493,37 @@ sml_getattr(struct worker *wrk, struct objcore *oc, enum obj_attr attr,
 		len = &dummy;
 	o = sml_getobj(wrk, oc);
 	CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
+
 	switch (attr) {
-	case OA_ESIDATA:
-		if (o->esidata == NULL)
-			return (NULL);
-		*len = o->esidata->len;
-		return (o->esidata->ptr);
-	case OA_FLAGS:
-		*len = sizeof o->oa_flags;
-		return (o->oa_flags);
-	case OA_GZIPBITS:
-		*len = sizeof o->oa_gzipbits;
-		return (o->oa_gzipbits);
-	case OA_HEADERS:
-		*len = 0;			// XXX: hack
-		return (o->oa_http);
-	case OA_LASTMODIFIED:
-		*len = sizeof o->oa_lastmodified;
-		return (o->oa_lastmodified);
-	case OA_VARY:
-		*len = 4;			// XXX: hack
-		return (o->oa_vary);
-	case OA_LEN:
-		*len = sizeof o->oa_len;
-		return (o->oa_len);
-	case OA_VXID:
-		*len = sizeof o->oa_vxid;
-		return (o->oa_vxid);
+		/* Fixed size attributes */
+#define OBJ_FIXATTR(U, l, s)						\
+	case OA_##U:							\
+		*len = sizeof o->fa_##l;				\
+		return (o->fa_##l);
+#include "tbl/obj_attr.h"
+#undef OBJ_FIXATTR
+
+		/* Variable size attributes */
+#define OBJ_VARATTR(U, l)						\
+	case OA_##U:							\
+		if (o->va_##l == NULL)					\
+			return (NULL);					\
+		*len = o->va_##l##_len;					\
+		return (o->va_##l);
+#include "tbl/obj_attr.h"
+#undef OBJ_VARATTR
+
+		/* Auxiliary attributes */
+#define OBJ_AUXATTR(U, l)						\
+	case OA_##U:							\
+		if (o->aa_##l == NULL)					\
+			return (NULL);					\
+		CHECK_OBJ_NOTNULL(o->aa_##l, STORAGE_MAGIC);		\
+		*len = o->aa_##l->len;					\
+		return (o->aa_##l->ptr);
+#include "tbl/obj_attr.h"
+#undef OBJ_AUXATTR
+
 	default:
 		break;
 	}
@@ -535,53 +543,63 @@ sml_setattr(struct worker *wrk, struct objcore *oc, enum obj_attr attr,
 	o = sml_getobj(wrk, oc);
 	CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
 	st = o->objstore;
+
 	switch (attr) {
-	case OA_ESIDATA:
-		o->esidata = objallocwithnuke(wrk, oc->stobj->stevedore, len);
-		if (o->esidata == NULL)
-			return (NULL);
-		o->esidata->len = len;
-		retval = o->esidata->ptr;
-		break;
-	case OA_FLAGS:
-		assert(len == sizeof o->oa_flags);
-		retval = o->oa_flags;
-		break;
-	case OA_GZIPBITS:
-		assert(len == sizeof o->oa_gzipbits);
-		retval = o->oa_gzipbits;
+		/* Fixed size attributes */
+#define OBJ_FIXATTR(U, l, s)						\
+	case OA_##U:							\
+		assert(len == sizeof o->fa_##l);			\
+		retval = o->fa_##l;					\
 		break;
-	case OA_HEADERS:
-		len = PRNDUP(len);
-		assert(st->len + len <= st->space);
-		o->oa_http = (void*)(st->ptr + st->len);
-		st->len += len;
-		retval = o->oa_http;
+#include "tbl/obj_attr.h"
+#undef OBJ_FIXATTR
+
+		/* Variable size attributes */
+#define OBJ_VARATTR(U, l)						\
+	case OA_##U:							\
+		if (o->va_##l##_len > 0) {				\
+			AN(o->va_##l);					\
+			assert(len == o->va_##l##_len);			\
+			retval = o->va_##l;				\
+		} else if (len > 0) {					\
+			assert(len <= UINT_MAX);			\
+			assert(st->len + len <= st->space);		\
+			o->va_##l = st->ptr + st->len;			\
+			st->len += len;					\
+			retval = o->va_##l;				\
+		}							\
 		break;
-	case OA_LASTMODIFIED:
-		assert(len == sizeof o->oa_lastmodified);
-		retval = o->oa_lastmodified;
-		break;
-	case OA_VARY:
-		len = PRNDUP(len);
-		assert(st->len + len <= st->space);
-		o->oa_vary = (void*)(st->ptr + st->len);
-		st->len += len;
-		retval = o->oa_vary;
-		break;
-	case OA_LEN:
-		assert(len == sizeof o->oa_len);
-		retval = o->oa_len;
-		break;
-	case OA_VXID:
-		assert(len == sizeof o->oa_vxid);
-		retval = o->oa_vxid;
+#include "tbl/obj_attr.h"
+#undef OBJ_VARATTR
+
+		/* Auxiliary attributes */
+#define OBJ_AUXATTR(U, l)						\
+	case OA_##U:							\
+		if (o->aa_##l != NULL) {				\
+			CHECK_OBJ_NOTNULL(o->aa_##l, STORAGE_MAGIC);	\
+			assert(len == o->aa_##l->len);			\
+			retval = o->aa_##l->ptr;			\
+			break;						\
+		}							\
+		if (len == 0)						\
+			break;						\
+		o->aa_##l = objallocwithnuke(wrk, oc->stobj->stevedore,	len); \
+		if (o->aa_##l == NULL)					\
+			break;						\
+		CHECK_OBJ_NOTNULL(o->aa_##l, STORAGE_MAGIC);		\
+		assert(len <= o->aa_##l->space);			\
+		o->aa_##l->len = len;					\
+		retval = o->aa_##l->ptr;				\
 		break;
+#include "tbl/obj_attr.h"
+#undef OBJ_AUXATTR
+
 	default:
 		WRONG("Unsupported OBJ_ATTR");
 		break;
 	}
-	if (ptr != NULL)
+
+	if (retval != NULL && ptr != NULL)
 		memcpy(retval, ptr, len);
 	return (retval);
 }
diff --git a/bin/varnishd/storage/storage_simple.h b/bin/varnishd/storage/storage_simple.h
index 84a3277..53d2b94 100644
--- a/bin/varnishd/storage/storage_simple.h
+++ b/bin/varnishd/storage/storage_simple.h
@@ -40,17 +40,29 @@ struct object {
 #define OBJECT_MAGIC		0x32851d42
 	struct storage		*objstore;
 
-	uint8_t			oa_len[8];
-	uint8_t			oa_vxid[4];
-	uint8_t			*oa_vary;
-	uint8_t			*oa_http;
-	uint8_t			oa_flags[1];
-	char			oa_gzipbits[32];
-	char			oa_lastmodified[8];
+	/* Fixed size attributes */
+#define OBJ_FIXATTR(U, l, s)			\
+	uint8_t			fa_##l[s];
+#include "tbl/obj_attr.h"
+#undef OBJ_FIXATTR
 
-	struct storagehead	list;
+	/* Variable size attributes */
+#define OBJ_VARATTR(U, l)			\
+	uint8_t			*va_##l;
+#include "tbl/obj_attr.h"
+#undef OBJ_VARATTR
+#define OBJ_VARATTR(U, l)			\
+	unsigned		va_##l##_len;
+#include "tbl/obj_attr.h"
+#undef OBJ_VARATTR
+
+	/* Auxiliary attributes */
+#define OBJ_AUXATTR(U, l)			\
+	struct storage		*aa_##l;
+#include "tbl/obj_attr.h"
+#undef OBJ_AUXATTR
 
-	struct storage		*esidata;
+	struct storagehead	list;
 };
 
 extern const struct obj_methods SML_methods;
@@ -59,4 +71,3 @@ struct object *SML_MkObject(const struct stevedore *, struct objcore *,
     void *ptr);
 
 storage_allocobj_f SML_allocobj;
-
diff --git a/include/tbl/obj_attr.h b/include/tbl/obj_attr.h
index 310ffab..d05b598 100644
--- a/include/tbl/obj_attr.h
+++ b/include/tbl/obj_attr.h
@@ -29,17 +29,24 @@
 
 /*lint -save -e525 -e539 */
 
+/* upper, lower, size */
+#ifdef OBJ_FIXATTR
+OBJ_FIXATTR(LEN, len, 8)
+OBJ_FIXATTR(VXID, vxid, 4)
+OBJ_FIXATTR(FLAGS, flags, 1)
+OBJ_FIXATTR(GZIPBITS, gzipbits, 32)
+OBJ_FIXATTR(LASTMODIFIED, lastmodified, 8)
+#endif
+
+/* upper, lower */
+#ifdef OBJ_VARATTR
+OBJ_VARATTR(VARY, vary)
+OBJ_VARATTR(HEADERS, headers)
+#endif
+
 /* upper, lower */
-#ifdef OBJ_ATTR
-OBJ_ATTR(LEN,		len)
-OBJ_ATTR(VXID,		vxid)
-OBJ_ATTR(EXP,		exp)
-OBJ_ATTR(VARY,		vary)
-OBJ_ATTR(HEADERS,	headers)
-OBJ_ATTR(FLAGS,		flags)
-OBJ_ATTR(GZIPBITS,	gzipbits)
-OBJ_ATTR(ESIDATA,	esidata)
-OBJ_ATTR(LASTMODIFIED,	lastmodified)
+#ifdef OBJ_AUXATTR
+OBJ_AUXATTR(ESIDATA, esidata)
 #endif
 
 #ifdef OBJ_FLAG



More information about the varnish-commit mailing list