mdb_v8 icon indicating copy to clipboard operation
mdb_v8 copied to clipboard

v8o_optional serves no purpose

Open mgerdts opened this issue 6 years ago • 0 comments

mdb_v8.c has:

typedef struct v8_offset {
	ssize_t		*v8o_valp;
	const char	*v8o_class;
	const char	*v8o_member;
	boolean_t	v8o_optional;
	uint32_t	v8o_flags;
	intptr_t	v8o_fallback;
} v8_offset_t;

v8o_flags has:

#define	V8_CONSTANT_OPTIONAL		1
#define	V8_CONSTANT_HASFALLBACK		2
#define	V8_CONSTANT_REMOVED		4
#define	V8_CONSTANT_ADDED		8

V8_CONSTANT_FALLBACK() is commonly used to indicate that a value is optional and that it has a fallback value by setting V8_CONSTANT_OPTIONAL, among other things. However, v8o_optional is set to non-zero and V8_CONSTANT_FALLBACK() is used, the fallback value will never be used. This is confusing.

Worse, there are some offsets that are improperly initialized such that v8o_optional is set to a non-zero value intended for v8o_flags. A proper v8_offset_t initializer is:

	{ &V8_OFF_MAP_INOBJECT_PROPERTIES_OR_CTOR_FUN_INDEX,
	    "Map", "inobject_properties_or_constructor_function_index",
	    B_FALSE, V8_CONSTANT_FALLBACK(4, 6), 4 },

Examples of improper initializers that omit the (required) v8o_optional field are:

	{ &V8_OFF_SHAREDFUNCTIONINFO_INFERRED_NAME,
	    "SharedFunctionInfo", "inferred_name",
		V8_CONSTANT_REMOVED_SINCE(5, 1) },
#ifdef _LP64
	{ &V8_OFF_SHAREDFUNCTIONINFO_IDENTIFIER,
	    "SharedFunctionInfo", "function_identifier",
		V8_CONSTANT_FALLBACK(5, 1), 79},
#else
	{ &V8_OFF_SHAREDFUNCTIONINFO_IDENTIFIER,
	    "SharedFunctionInfo", "function_identifier",
		V8_CONSTANT_FALLBACK(5, 1), 39},
#endif

It is understandable that someone made this mistake. v8_constants[] uses very similar initializers but does not have an equivalent of the v8o_optional field.

Code inspection and testing suggests that v8o_optional should go away.

mgerdts avatar Jun 04 '19 03:06 mgerdts