<div dir="ltr">Three few more patches. Hope these are better done.<div><br></div><div style>B.</div><div style><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, May 18, 2013 at 3:07 PM, Sylvain Munaut <span dir="ltr"><<a href="mailto:246tnt@gmail.com" target="_blank">246tnt@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Sat, May 18, 2013 at 11:13 AM, Holger Hans Peter Freyther<br>
<<a href="mailto:holger@freyther.de">holger@freyther.de</a>> wrote:<br>
> On Sat, May 18, 2013 at 10:59:25AM +0530, Bhaskar11 wrote:<br>
>> Two more bugfix patches. Please give me feedback if these patches are<br>
>> working ok.<br>
><br>
> thanks, i will attempt to review/merge them over the next couple<br>
> of days.<br>
<br>
<br>
</div></div>I had a quick look at the last two, but I'm not convinced, I'll<br>
probably rewrite a fix for those.<br>
<br>
1) There is actually 3 places where trans_assign_trans_id is used and<br>
the one that's correct uses 'int' and not 'int8_t' and this seems a<br>
better approach because that's the native return type of the<br>
trans_assign_trans_id function. This way we have 'int' (return value<br>
allowing full valid range of uint8_t + negative values for error) and<br>
'uint8_t' ( final valid value stored in structure ), and we avoid<br>
going from int -> int8_t -> uint8_t. And as a bonus the three places<br>
in the code with this pattern are now consistent with each other.<br>
<br>
2) The patch title is wrong :<br>
  - It actually says "should be unsigned int to allow negative values"<br>
  - It's also too long and doesn't include an indication of the<br>
subsystem it refers to.<br>
<br>
3) IMHO, Those two patches are trivial and similar enough to have been<br>
made in one commit.<br>
<br>
I've attached as an example the corrected patch I will apply, for<br>
future reference.<br>
<br>
I'll probably get to the other later in the day.<br>
<br>
Cheers,<br>
<br>
    Sylvain<br>
</blockquote></div><br></div>