enumtypes: skip generation of "last" members #4520
Conversation
|
I suppose we could also just remove the https://docs.gtk.org/gobject/struct.EnumClass.html We wouldn't be able to get the size of the enum statically, but maybe that's not too bad? ruby-vips, php-vips etc might need changes too. Maybe that's too big a change. |
|
I think removing the |
| /* -1 since we always have a "last" member. | ||
| */ | ||
| for (i = 0; i < genum->n_values - 1; i++) { | ||
| for (i = 0; i < genum->n_values; i++) { |
There was a problem hiding this comment.
Does this change mean that the "last" string literal will no longer accepted by the vips_enum_from_nick function? (There are a couple of places in sharp that rely on this behaviour, which I'm happy to remove if so as it's probably not a good thing to rely on anyway.)
There was a problem hiding this comment.
Good spot. Yes, this would mean that the "last" string literal will no longer be accepted. See for example:
/* Compile with:
* gcc -g -Wall test-enum.c `pkg-config vips --cflags --libs`
*/
#include <vips/vips.h>
int
main(int argc, char *argv[]) {
if (VIPS_INIT(argv[0]))
vips_error_exit(NULL);
int i = vips_enum_from_nick(argv[0], VIPS_TYPE_INTERPRETATION, "last");
if (i == -1)
// Error: `enum 'VipsInterpretation' has no member 'last', should be one of: ...`
vips_error_exit(NULL);
// This would previously print 30 (VIPS_INTERPRETATION_LAST), rather than bailing out.
printf("%d\n", i);
return 0;
}|
I checked NetVips, php-vips, pyvips, and ruby-vips, none of these language bindings appear to expose these My only concern is the potential $ vipsthumbnail zebra.jpg --smartcrop=last
Segmentation fault (core dumped)due to: libvips/libvips/conversion/smartcrop.c Line 394 in d87f9ed But with this PR, it would error with: $ vipsthumbnail zebra.jpg --smartcrop=last
vipsthumbnail: unable to thumbnail zebra.jpg
vipsthumbnail: enum 'VipsInteresting' has no member 'last', should be one of: none, centre, entropy, attention, low, high, all |
|
As an experiment, here's a branch that removes these |
|
I think we'd need something for uses like: if (mode[i] < 0 ||
mode[i] >= VIPS_BLEND_MODE_LAST) {Maybe: // tiny utility func
GEnumClass *vips__get_enum_class(const char *class_name);
#define VIPS_BLEND_MODE_LAST (vips__get_enum_class("VipsBlendMode")->n_values)Or perhaps just: // returns n_values
int vips__get_enum_last(const char *class_name);
if (mode[i] < 0 ||
mode[i] > vips__get_enum_last("VipsBlendMode")) {What do you think? |
|
You're right. I just pushed commit kleisauke@1f0054e to that branch, since that's probably(?) the only place where it needs to be future-proof. Note that we sometimes do similar checks elsewhere, for example: libvips/libvips/iofuncs/image.c Line 670 in 9f590ce which is likely safe, I don't expect any new band formats in the near future. |
We probably don't want to expose these "last" members to language bindings.
Generated
enumtypes.cdiff: https://gist.github.com/kleisauke/97d3e7d01df0a79b4b9179ea201e053eSee also: libvips/pyvips#537.