Please review patch for aic7xxx_pci.c

[Justin T. Gibbs]

On Jul 23, 2013, at 4:29 PM, Ulrich Spörlein <uqs at freebsd.org> wrote:

> On Tue, 2013-07-23 at 16:04:04 -0600, Justin T. Gibbs wrote:
>> On Jul 23, 2013, at 3:41 PM, Ulrich Spörlein <uqs at freebsd.org> wrote:
>> 
>>> On Tue, 2013-07-23 at 15:15:53 -0600, Justin T. Gibbs wrote:
>>>> On Jul 23, 2013, at 2:45 PM, Ulrich Spörlein <uqs at freebsd.org> wrote:
>>>> 
>>>>> Thanks, but maybe it's better to have the discussion here.
>>>>> 
>>>>> You wrote:
>>>>> The difference in the argument order between the declaration and definition is
>>>>> likely what led to this programming error. Your patch should also correct the
>>>>> argument reversal in the declaration of ahc_9005_subdevinfo_valid().
>>>>> 
>>>>> The argument order you've used seems the best choice since it matches what is
>>>>> used in ahc_compose_id().
>>>>> 
>>>>> 
>>>>> 
>>>>> I'm not sure that reversing the arguments in the function definition is
>>>>> the right way. This would make the patch a no-op.
>>>> 
>>>> Read the quote again.  I said change the "declaration", not the
>>>> "definition".  To be extremely clear, the code currently looks like
>>>> this:
>>> 
>>> D'oh, that's what I get for reading too fast. I've opted to change the
>>> function definition, in the end, so that declaration, definition and
>>> call-site are in sync, see
>>> https://github.com/uqs/freebsd-head/commit/e699b2fde01f235a22e1125e9f3aef16623b512f
>>> 
>>> Thanks!
>>> Uli
>> 
>> But now the order of the arguments does not match ahc_compose_id().
>> 
>> It seems to me that we want to only have a single convention in how this
>> data is passed to functions.
> 
> Ok, back to the first variant then.

Works for me.

--
Justin