On Tue, 2005-07-05 at 16:33, Roland Dreier wrote:
>     Hal> Right. It is a convenient define for a consumer using Service
>     Hal> Records. I would prefer to leave it in there despite it not
>     Hal> being used.
> 
> OK, that's fine.
> 
>     Hal> It does map to the actual SR layout and path record indicates
>     Hal> where the reserved fields are.
> 
>     Hal> Is changing it to /* reserved */ acceptable like in the
>     Hal> ib_sa_path_rec struct declaration ?
> 
> Actually, I didn't read the patch carefully enough.  Yes, please
> follow the style used in the rest of the file: name the component mask
> entries as they are named in the spec, rather than using your own
> abbreviations, so you have something like
> 
> #define IB_SA_SERVICE_REC_SERVICE_ID                  IB_SA_COMP_MASK( 0)
> #define IB_SA_SERVICE_REC_SERVICE_GID                 IB_SA_COMP_MASK( 1)
> #define IB_SA_SERVICE_REC_SERVICE_PKEY                        
> IB_SA_COMP_MASK( 2)
> /* reserved:                                                           3 */
> #define IB_SA_SERVICE_REC_SERVICE_LEASE                       
> IB_SA_COMP_MASK( 4)
> 
> and so on.  And there's no need to define a component mask for reserved rows.

I think there is a legitimate use for this: when one wants to set a lot
of component mask bits and wants to turn off the reserved bit (rather
than or'ing a long list together).

Please verify that the style is acceptable in the next version of the
patch.

> Then you can put 
> 
>       /* reserved */
> 
> in the structure definition to match the component mask.

OK.

-- Hal

_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to