Code Quality: Let GHash/GSet comparison callbacks return true on equality, not false #91870

New Issue

Julian Eisel · 2021-10-01T12:53:39+02:00

Julian Eisel commented

2021-10-01 12:53:39 +02:00

For historical reasons (compatibility with C library compare functions such as strcmp), the comparison functions of GHash (GHashCmpFP) and GSet (GSetCmpFP) are expected to return false if the items match, not true.
This is confusing and unintuitive and in fact has led to bugs in the past, where this was done the wrong way around: f2b86471ea

What needs to be done

Code using the compare functions has to be corrected.
Comments needs to be updated.
All comparison callbacks have to be updated. (This is the biggest part of the work.)
Make sure the individual comparison callbacks are not used for other logic. If so, that needs to be updated.

For historical reasons (compatibility with C library compare functions such as `strcmp`), the comparison functions of `GHash` (`GHashCmpFP)` and `GSet` (`GSetCmpFP`) are expected to return `false` if the items match, not `true`. This is confusing and unintuitive and in fact has led to bugs in the past, where this was done the wrong way around: f2b86471ea ### What needs to be done * Code using the compare functions has to be corrected. * Comments needs to be updated. * All comparison callbacks have to be updated. (This is the biggest part of the work.) * Make sure the individual comparison callbacks are not used for other logic. If so, that needs to be updated.

Julian Eisel commented

2021-10-01 12:53:39 +02:00

Added subscriber: @JulianEisel

Ray molenkamp commented

2021-12-06 19:16:24 +01:00

Added subscriber: @LazyDodo

Ray molenkamp commented

2021-12-06 19:16:24 +01:00

I feel you can go further here and rename the callbacks them selves to be more indicative of the type and expected behavior, ie GHashCompareEqualityCallback over GHashCmpFP
.

I feel you can go further here and rename the callbacks them selves to be more indicative of the type and expected behavior, ie `GHashCompareEqualityCallback` over `GHashCmpFP` .

Philipp Oeser commented

2022-02-16 16:35:45 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2022-02-16 16:35:45 +01:00

Changed status from 'Needs Triage' to: 'Needs Developer To Reproduce'

Philipp Oeser commented

2022-02-16 16:35:45 +01:00

Not sure if this is still in the decision phase?
Will change status since there is probably not much the #triaging team can do here.

Not sure if this is still in the decision phase? Will change status since there is probably not much the #triaging team can do here.

Bastien Montagne commented

2022-02-16 16:54:54 +01:00

Changed status from 'Needs Developer To Reproduce' to: 'Confirmed'

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Code Quality: Let GHash/GSet comparison callbacks return true on equality, not false #91870

What needs to be done