On Wed, Dec 17, 2014 at 3:31 PM, Martin Storsjö <[email protected]> wrote:
> On Wed, 17 Dec 2014, Vittorio Giovara wrote:
>
>> CC: [email protected]
>> Bug-Id: CID 732285 / CID 732286
>> ---
>> libavcodec/aacps.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/aacps.c b/libavcodec/aacps.c
>> index 8f55c7f..df069c3 100644
>> --- a/libavcodec/aacps.c
>> +++ b/libavcodec/aacps.c
>> @@ -139,7 +139,7 @@ static int ps_read_extension_data(GetBitContext *gb,
>> PSContext *ps, int ps_exten
>>     return get_bits_count(gb) - count;
>> }
>>
>> -static void ipdopd_reset(int8_t *opd_hist, int8_t *ipd_hist)
>> +static void ipdopd_reset(int8_t *ipd_hist, int8_t *opd_hist)
>> {
>>     int i;
>>     for (i = 0; i < PS_MAX_NR_IPDOPD; i++) {
>> --
>> 1.9.3 (Apple Git-50)
>
>
> The commit message needs expanding - on its own this change doesn't look
> like it makes much sense, unless you say e.g. that this is the order that
> the caller expects it to be, or whatever the issue is.
>
> In general, unless it's absolutely crystal clear what issue coverity is
> complaining about, please mention it in the commit message, don't just
> mention an opaque CID. (Remember, commit messages should make sense on their
> own and external references should just be an extra; all the vital
> information shouldn't be in the external references.) Without reading the
> rest of the file, it's impossible to judge from just reading this one single
> patch what it is about.
>
> Also, the things you swap are parameters, not operands. The comma inbetween
> is not the comma operator.

You are right. Would something like the following be better instead?

aacps: invert the order of parameters of ipdopd_reset()

This is the order that the caller uses in the rest of the file.

Bug-Id [...]

Thanks.
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to