Re: [PATCH] util: virTimeFieldsThenRaw never return negative

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

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(...);

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.

>     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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list