-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Float and null type support #197
Conversation
…value iterator and parameter tests
@@ -95,7 +106,7 @@ ddwaf_object *ddwaf_object_stringl_nc(ddwaf_object *object, const char *string, | |||
return object; | |||
} | |||
|
|||
ddwaf_object *ddwaf_object_signed(ddwaf_object *object, int64_t value) | |||
ddwaf_object *ddwaf_object_string_from_signed(ddwaf_object *object, int64_t value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP: I like this new naming from
but it feels a bit inconsistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How so?
double ddwaf_object_get_float(const ddwaf_object *object) | ||
{ | ||
if (object == nullptr || object->type != DDWAF_OBJ_FLOAT) { | ||
return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do user of this function know if the 0 comes from the object or from this conditional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They can't know, although they can always check if their object is null or of an incorrect type, but it's mostly a precaution for incorrect use of the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nitpicks
Add support for
float
andnull
types, while also allowinginvalid
within containers.Breaking changes:
ddwaf_object
now has one more field in the union for doubles calledf64
ddwaf_object_*_force
are now the standard, so the_force
suffix has been removed.ddwaf_object_signed
andddwaf_object_unsigned
have now been renamed toddwaf_object_string_from_*
DDWAF_OBJ_FLOAT
andDDWAF_OBJ_NULL