[ofa-general] Re: [PATCH 1/7] osm: QoS - adding new PathRecord fields

[Sasha Khapyorsky]

Hi Yevgeny,

On 14:11 Tue 21 Aug     , Yevgeny Kliteynik wrote:
> >>  *
> >>  * RETURN VALUES
> >> -*	Encoded path pkt_life = 4.096 ?sec * 2 ** PacketLifeTime.
> >> +*	Encoded path pkt_life = 4.096 ??sec * 2 ** PacketLifeTime.
> > It is not ascii. What is the change here?
> 
>  I think my diff tool has troubles handling unusual chars.
>  There's a '?' character there - it wasn't changed.
>  I guess it would be better to replace it with "micro" in
>  the future - less elegant, but also less error prone.

Yes, just micro or usec look fine.

> > Also it would be nice if newly added code will be formatted with
> > opensm/osm_indent. Thanks.
> 
>  To tell you the truth, I'm not a big fan of this script.
>  Sometimes it does stuff it shouldn't. For instance,
>  I run it on osm_qos_policy.c, and this is what it does:
> 
>  Before:
> 
>  static osm_qos_match_rule_t *
>  __qos_policy_get_match_rule_by_pr(
>      const osm_pr_rcv_t   * p_rcv,
>      const ib_path_rec_t  * p_pr,
>      const osm_physp_t    * p_src_physp,
>      const osm_physp_t    * p_dest_physp,
>      ib_net64_t             comp_mask)
> 
> 
>  After:
> 
>  static osm_qos_match_rule_t *__qos_policy_get_match_rule_by_pr(const
>  							       osm_pr_rcv_t *
>  							       p_rcv,
>  							       const
>  							       ib_path_rec_t *
>  							       p_pr,
>  							       const osm_physp_t
>  							       * p_src_physp,
>  							       const osm_physp_t
>  							       * p_dest_physp,
>  							       ib_net64_t
>  							       comp_mask)
> 
>  Correct me if I'm wrong, but I think that the 'before' example
>  is closer to our coding style than the 'after'.

Sure, indent program can do stupid things sometimes (mostly when fooled
by long names). But main idea was to have consistent and readable
source code. So both examples are bad from this point of view.

Sasha