On 2014/7/24 13:18, Martin Kletzander wrote:
> On Thu, Jul 24, 2014 at 10:17:45AM +0800, Wang Rui wrote:
>> From: James <james.wangyufei@xxxxxxxxxx>
>>
>> virTimeFieldsThenRaw will never return negative result, so I delete related
>> judgement.
>>
>
> Looking at the code I think it was pretty nicely prepared for error
> reporting, but then there was no error that could happen in the
> function, so this cleanup makes sense.
>
>> Signed-off-by: James <james.wangyufei@xxxxxxxxxx>
>> ---
>> src/util/virtime.c | 7 ++-----
>> src/util/virtime.h | 4 ++--
>> 2 files changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/util/virtime.c b/src/util/virtime.c
>> index 2a91ea5..662c161 100644
>> --- a/src/util/virtime.c
>> +++ b/src/util/virtime.c
>> @@ -121,9 +121,8 @@ const unsigned short int __mon_yday[2][13] = {
>> * Converts the timestamp @when into broken-down field format.
>> * Time time is always in UTC
>> *
>> - * Returns 0 on success, -1 on error with errno set
>> */
>> -int virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
>> +void virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
>> {
>> /* This code is taken from GLibC under terms of LGPLv2+ */
>> long int days, rem, y;
>> @@ -171,7 +170,6 @@ int virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
>> days -= ip[y];
>> fields->tm_mon = y;
>> fields->tm_mday = days + 1;
>> - return 0;
>> }
>>
>>
>> @@ -209,8 +207,7 @@ int virTimeStringThenRaw(unsigned long long when, char *buf)
>> {
>> struct tm fields;
>>
>> - if (virTimeFieldsThenRaw(when, &fields) < 0)
>> - return -1;
>> + virTimeFieldsThenRaw(when, &fields);
>>
>
> This is OK, but there's a virTimeFieldsNowRaw() function that does:
> return virTimeFieldsThenRaw(...);
Thanks for your review.
You are right. Function virTimeFieldsNowRaw() should return 0 in case of success.
>
> That should be fixed as well. And virTimeFieldsThen() encapsulates
> the Raw version with an error message that's pointless now. So that
> function can be void too. The whole chain of function calls should be
> fixed up, not just one call to it.
>
OK, that will be fixed in V2.
>> fields.tm_year += 1900;
>> fields.tm_mon += 1;
>> diff --git a/src/util/virtime.h b/src/util/virtime.h
>> index 25332db..61f36dc 100644
>> --- a/src/util/virtime.h
>> +++ b/src/util/virtime.h
>> @@ -43,12 +43,12 @@ int virTimeMillisNowRaw(unsigned long long *now)
>> ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>> int virTimeFieldsNowRaw(struct tm *fields)
>> ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>> -int virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
>> - ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
>> int virTimeStringNowRaw(char *buf)
>> ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>> int virTimeStringThenRaw(unsigned long long when, char *buf)
>> ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
>> +void virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
>> + ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
>>
>
> No need to move this; I think it's probably sorted by function name or
> it has the same order as the .c file.
>
> Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list