[libvirt] [PATCH] util: virTimeFieldsThenRaw never return negative
From: James <james.wangyufei@huawei.com> virTimeFieldsThenRaw will never return negative result, so I delete related judgement. Signed-off-by: James <james.wangyufei@huawei.com> --- 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); 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; /* These APIs are *not* async signal safe and return -1, * raising a libvirt error on failure -- 1.7.12.4
On Thu, Jul 24, 2014 at 10:17:45AM +0800, Wang Rui wrote:
From: James <james.wangyufei@huawei.com>
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@huawei.com> --- 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
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@huawei.com>
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@huawei.com> --- 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
participants (2)
-
Martin Kletzander -
Wang Rui