Re: [PATCH v2 07/11] flow: Introduce 'sidx' type to represent one side of one flow

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 30 Nov 2023 11:37:40 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Nov 29, 2023 at 03:32:32PM +0100, Stefano Brivio wrote:
> > On Mon, 27 Nov 2023 10:33:44 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > In a number of places, we use indices into the flow table to identify a
> > > specific flow.  We also have cases where we need to identify a particular
> > > side of a particular flow, and we expect those to become more common as
> > > we generalise the flow table to cover more things.
> > > 
> > > To assist with that, introduces flow_sidx_t, an index type which identifies
> > > a specific side of a specific flow in the table.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > >  flow.h       | 13 +++++++++++++
> > >  flow_table.h | 36 ++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 49 insertions(+)
> > > 
> > > diff --git a/flow.h b/flow.h
> > > index b6da516..3c90bbd 100644
> > > --- a/flow.h
> > > +++ b/flow.h
> > > @@ -39,6 +39,19 @@ struct flow_common {
> > >  #define FLOW_TABLE_PRESSURE		30	/* % of FLOW_MAX */
> > >  #define FLOW_FILE_PRESSURE		30	/* % of c->nofile */
> > >  
> > > +/**
> > > + * struct flow_sidx - ID for one side of a specific flow
> > > + * @side:	Side referenced (0 or 1)
> > > + * @flow:	Index of flow referenced
> > > + */
> > > +typedef struct flow_sidx {  
> > 
> > Implying my usual argument :) ...is there any advantage over using this
> > simply as a struct?  
> 
> So, usually I too would prefer to use a struct as a struct, without a
> typedef.  The reason I'm doing differently here, is that I want to
> emphasise that for many purposes this can be treated like an index, in
> particular that it's small and trivially copyable.  In particular it
> should be passed by value, passing by reference would be silly.

Hmm, that was exactly my "not hiding" point though. The day somebody
adds here:

	char mood[RLIMIT_STACK_VAL + 1]; /* list of side emojis */

the typedef makes it still apparently okay to pass by value. If it's a
struct, one surely has to check first.

> That's kind of the opposite of what one tends to be conveying by
> reminding users that they're working with a struct.

I see, but it's probably a matter of taste (passing structs by value
doesn't personally make me nervous).

-- 
Stefano