Re: [XEN PATCH 3/5] xen/sort: address violations of MISRA C:2012 Rule 8.2

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 22/11/23 09:01, Jan Beulich wrote:
> On 22.11.2023 02:19, Stefano Stabellini wrote:
> > On Tue, 21 Nov 2023, Jan Beulich wrote:
> > > On 21.11.2023 01:04, Stefano Stabellini wrote:
> > > > On Mon, 20 Nov 2023, Jan Beulich wrote:
> > > > > On 20.11.2023 14:13, Federico Serafini wrote:
> > > > > > On 20/11/23 10:02, Jan Beulich wrote:
> > > > > > > On 17.11.2023 09:40, Federico Serafini wrote:
> > > > > > > > --- a/xen/include/xen/sort.h
> > > > > > > > +++ b/xen/include/xen/sort.h
> > > > > > > > @@ -23,8 +23,8 @@
> > > > > > > >    extern gnu_inline
> > > > > > > >    #endif
> > > > > > > >    void sort(void *base, size_t num, size_t size,
> > > > > > > > -          int (*cmp)(const void *, const void *),
> > > > > > > > -          void (*swap)(void *, void *, size_t))
> > > > > > > > +          int (*cmp)(const void *key, const void *elem),
> > > > > > > Why "key" and "elem" here, but ...
> > > > > > >
> > > > > > > > +          void (*swap)(void *a, void *b, size_t size))
> > > > > > > ... "a" and "b" here? The first example of users of sort() that I'm
> > > > > > > looking at right now (x86/extable.c) is consistent in its naming.
> > > > > > On the Arm side there are {cmp,swap}_memory_node() and
> > > > > > {cmp,swap}_mmio_handler(): "key"/"elem" are used for the comparison
> > > > > > and "_a"/"_b" for the swap.
> > > > > So - re-raising a question Stefano did raise - is Misra concerned about
> > > > > such discrepancies? If yes, _all_ instances need harmonizing. If not, I
> > > > > see no reason to go with misleading names here.
> > > > Federico confirmed that the answer is "no".
> > > >
> > > > I think we can use "key" and "elem" in this patch as they are more
> > > > informative than "a" and "b"
> > > Except that "key" and "elem" are (imo) inapplicable to sort() callbacks
> > > (and inconsistent with the naming in the 2nd callback here); they _may_
> > > be applicable in bsearch() ones. Note also how in the C99 spec these
> > > parameters of callback functions don't have names either.
> > Yes, reading the example in extable.c I think you are right. Maybe it is
> > better to use "a" and "b" in both cmp and swap if you agree.
> Using a and b is (as it looks) in line with at least some uses we have, so
> less code churn than going with some other, more descriptive names (like
> left/right). So yes, I'm okay with using a/b.
I'll send a new version.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)