On Sat, May 18, 2013 at 11:13 AM, Holger Hans Peter Freyther
<holger@freyther.de> wrote:
> On Sat, May 18, 2013 at 10:59:25AM +0530, Bhaskar11 wrote:
>> Two more bugfix patches. Please give me feedback if these patches are
>> working ok.
>
> thanks, i will attempt to review/merge them over the next couple
> of days.

I had a quick look at the last two, but I'm not convinced, I'll
probably rewrite a fix for those.

1) There is actually 3 places where trans_assign_trans_id is used and
the one that's correct uses 'int' and not 'int8_t' and this seems a
better approach because that's the native return type of the
trans_assign_trans_id function. This way we have 'int' (return value
allowing full valid range of uint8_t + negative values for error) and
'uint8_t' ( final valid value stored in structure ), and we avoid
going from int -> int8_t -> uint8_t. And as a bonus the three places
in the code with this pattern are now consistent with each other.

2) The patch title is wrong :
  - It actually says "should be unsigned int to allow negative values"
  - It's also too long and doesn't include an indication of the
subsystem it refers to.

3) IMHO, Those two patches are trivial and similar enough to have been
made in one commit.

I've attached as an example the corrected patch I will apply, for
future reference.

I'll probably get to the other later in the day.

Cheers,

    Sylvain