Updated MR to restore the sentinel value to the array returned by `openconnect_get_supported_protocols`, and also verified that the Java library was *not* affected, insofar as it used the length returned rather than relying on the sentinel value. -Dan On Mon, Apr 20, 2020 at 11:14 AM Daniel Lenski <dlenski@xxxxxxxxx> wrote: > > On Mon, Apr 20, 2020 at 11:05 AM David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote: > > Shows up if you run in valgrind though: > > Yes indeed. Wondering if there's a simple way to add automated > valgrind testing à la Coverity. > > > > I've created a MR to properly fix this: > > > https://gitlab.com/openconnect/openconnect/-/merge_requests/94 > > > > Nah, if openconnect_get_supported_protocols() originally returned an > > array with the sentinel included, then that is its ABI. We broke that. > > Right. I was thinking we might get away with it due to the small > number of (known) users, but not worth it. > > > Fixing up our own usage of that ABI doesn't actually excuse changing > > the ABI. And yes, one of the two callers in NetworkManager-openconnect > > does depend on the sentinel. We have to put it back. > > Makes sense. Will update the MR. > > > I think we *can* get away with returning the correct value (e.g. 4 now) > > rather than the 5 we used to return before commit 7cb8996e21. Even > > though that's strictly an ABI change it shoujld be OK. > > Agreed, that was basically an off-by-1 bug by me. Hopefully no one is > has reclassified it as an off-by-1 feature. :-P > > Dan _______________________________________________ openconnect-devel mailing list openconnect-devel@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/openconnect-devel
