On 05/10/2021 18:49, Morgan Adamiec wrote:
>
>
> On 05/10/2021 18:10, Andrew Gregory wrote:
>> On 10/05/21 at 12:53pm, Morgan Adamiec wrote:
>>> On 4 Oct 2021 8:28 pm, Andrew Gregory <[email protected]>
>>> wrote:
>>>
>>>
>>> On 10/04/21 at 08:09pm, morganamilo wrote:
>>>
>>> > This is the error value generally used and the calling function
>>>
>>> > explicitly checks for -1, later causing the error to be missed
>>>
>>> > and the transaction to continue.
>>>
>>>
>>>
>>> This result is not compared to -1, the result of download_files is.
>>> If
>>> we want
>>>
>>> to guarantee that download_files will return -1 on error, that's where
>>>
>>> the
>>>
>>> return should be normalized, not in find_dl_candidates. Tying the API
>>>
>>> of one
>>>
>>> function to another like this is just going to cause confusion and
>>>
>>> breakage
>>>
>>> when somebody forgets in the future. Really, the caller of
>>>
>>> download_files
>>>
>>> should just check for a successful return; we return 1 as an error
>>> from
>>> lots of
>>>
>>> functions.
>>>
>>>
>>>
>>> I'll change that too. This should still be accepted though.
>>>
>>
>> Why? If your reasoning is just that -1 is a better error value, we use 1 in
>> lots of other places like I said and I don't want to change that one at a
>> time.
>>
>> $ grep 'return 1;' lib/libalpm/*.c src/*/*.c | wc -l
>>
>>
>> [0][1016]
>> 132
>>
>
> Everywhere in the function returns -1. Lets at least be consistent for
> the same function.
>
Not to mention download_files returns 1 on everything up to date so 1 is
not an error in this case.
