Looks good. Thanks for addressing it cleanly.
On Thu, May 2, 2013 at 4:16 PM, Ben Pfaff <b...@nicira.com> wrote:
> strftime() returns 0 and leaves the contents of the output buffer
> unspecified if the output buffer is not big enough. Thus, one should
> check strftime()'s return value. Until now, OVS has had a few invocations
> of strftime() that did not check the return value. This commit fixes
> those. I believe that the buffers were always large enough in each case,
> but it's better to be safe.
>
> Reported-by: Andy Zhou <az...@nicira.com>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/dynamic-string.c | 23 +++++++++++++++++------
> lib/dynamic-string.h | 12 +++++++-----
> lib/table.c | 18 +++++++-----------
> lib/vlog.c | 11 ++++-------
> ovsdb/ovsdb-tool.c | 8 +++-----
> utilities/ovs-ofctl.c | 8 ++------
> 6 files changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c
> index c373601..ba9aa6d 100644
> --- a/lib/dynamic-string.c
> +++ b/lib/dynamic-string.c
> @@ -183,17 +183,16 @@ ds_put_printable(struct ds *ds, const char *s,
> size_t n)
> }
> }
>
> -/* Writes the current time to 'string' based on 'template'.
> - * The current time is either local time or UTC based on 'utc'. */
> +/* Writes time 'when' to 'string' based on 'template', in local time or
> UTC
> + * based on 'utc'. */
> void
> -ds_put_strftime(struct ds *ds, const char *template, bool utc)
> +ds_put_strftime(struct ds *ds, const char *template, time_t when, bool
> utc)
> {
> struct tm tm;
> - time_t now = time_wall();
> if (utc) {
> - gmtime_r(&now, &tm);
> + gmtime_r(&when, &tm);
> } else {
> - localtime_r(&now, &tm);
> + localtime_r(&when, &tm);
> }
>
> for (;;) {
> @@ -207,6 +206,18 @@ ds_put_strftime(struct ds *ds, const char *template,
> bool utc)
> }
> }
>
> +/* Returns a malloc()'d string for time 'when' based on 'template', in
> local
> + * time or UTC based on 'utc'. */
> +char *
> +xastrftime(const char *template, time_t when, bool utc)
> +{
> + struct ds s;
> +
> + ds_init(&s);
> + ds_put_strftime(&s, template, when, utc);
> + return s.string;
> +}
> +
> int
> ds_get_line(struct ds *ds, FILE *file)
> {
> diff --git a/lib/dynamic-string.h b/lib/dynamic-string.h
> index 098caaf..b988e1f 100644
> --- a/lib/dynamic-string.h
> +++ b/lib/dynamic-string.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -22,10 +22,9 @@
> #include <stddef.h>
> #include <stdint.h>
> #include <stdio.h>
> +#include <time.h>
> #include "compiler.h"
>
> -struct tm;
> -
> /* A "dynamic string", that is, a buffer that can be used to construct a
> * string across a series of operations that extend or modify it.
> *
> @@ -56,14 +55,17 @@ void ds_put_format(struct ds *, const char *, ...)
> PRINTF_FORMAT(2, 3);
> void ds_put_format_valist(struct ds *, const char *, va_list)
> PRINTF_FORMAT(2, 0);
> void ds_put_printable(struct ds *, const char *, size_t);
> -void ds_put_strftime(struct ds *, const char *, bool utc)
> - STRFTIME_FORMAT(2);
> void ds_put_hex_dump(struct ds *ds, const void *buf_, size_t size,
> uintptr_t ofs, bool ascii);
> int ds_get_line(struct ds *, FILE *);
> int ds_get_preprocessed_line(struct ds *, FILE *);
> int ds_get_test_line(struct ds *, FILE *);
>
> +void ds_put_strftime(struct ds *, const char *template, time_t when, bool
> utc)
> + STRFTIME_FORMAT(2);
> +char *xastrftime(const char *template, time_t when, bool utc)
> + STRFTIME_FORMAT(1);
> +
> char *ds_cstr(struct ds *);
> const char *ds_cstr_ro(const struct ds *);
> char *ds_steal_cstr(struct ds *);
> diff --git a/lib/table.c b/lib/table.c
> index 266c410..21f4905 100644
> --- a/lib/table.c
> +++ b/lib/table.c
> @@ -218,22 +218,19 @@ table_print_table_line__(struct ds *line)
> ds_clear(line);
> }
>
> -static void
> -table_format_timestamp__(char *s, size_t size)
> +static char *
> +table_format_timestamp__(void)
> {
> - time_t now = time_wall();
> - struct tm tm;
> - strftime(s, size, "%Y-%m-%d %H:%M:%S", gmtime_r(&now, &tm));
> + return xastrftime("%Y-%m-%d %H:%M:%S", time_wall(), true);
> }
>
> static void
> table_print_timestamp__(const struct table *table)
> {
> if (table->timestamp) {
> - char s[32];
> -
> - table_format_timestamp__(s, sizeof s);
> + char *s = table_format_timestamp__();
> puts(s);
> + free(s);
> }
> }
>
> @@ -500,10 +497,9 @@ table_print_json__(const struct table *table, const
> struct table_style *style)
> json_object_put_string(json, "caption", table->caption);
> }
> if (table->timestamp) {
> - char s[32];
> -
> - table_format_timestamp__(s, sizeof s);
> + char *s = table_format_timestamp__();
> json_object_put_string(json, "time", s);
> + free(s);
> }
>
> headings = json_array_create_empty();
> diff --git a/lib/vlog.c b/lib/vlog.c
> index 8bc9938..d53491c 100644
> --- a/lib/vlog.c
> +++ b/lib/vlog.c
> @@ -570,12 +570,9 @@ vlog_init(void)
>
> now = time_wall();
> if (now < 0) {
> - struct tm tm;
> - char s[128];
> -
> - gmtime_r(&now, &tm);
> - strftime(s, sizeof s, "%a, %d %b %Y %H:%M:%S", &tm);
> + char *s = xastrftime("%a, %d %b %Y %H:%M:%S", now, true);
> VLOG_ERR("current time is negative: %s (%ld)", s, (long int) now);
> + free(s);
> }
>
> unixctl_command_register(
> @@ -709,11 +706,11 @@ format_log_message(const struct vlog_module *module,
> enum vlog_level level,
> break;
> case 'd':
> p = fetch_braces(p, "%Y-%m-%d %H:%M:%S", tmp, sizeof tmp);
> - ds_put_strftime(s, tmp, false);
> + ds_put_strftime(s, tmp, time_wall(), false);
> break;
> case 'D':
> p = fetch_braces(p, "%Y-%m-%d %H:%M:%S", tmp, sizeof tmp);
> - ds_put_strftime(s, tmp, true);
> + ds_put_strftime(s, tmp, time_wall(), true);
> break;
> case 'm':
> /* Format user-supplied log message and trim trailing
> new-lines. */
> diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
> index fe6ab42..4d7d4f4 100644
> --- a/ovsdb/ovsdb-tool.c
> +++ b/ovsdb/ovsdb-tool.c
> @@ -519,11 +519,9 @@ do_show_log(int argc, char *argv[])
> date = shash_find_data(json_object(json), "_date");
> if (date && date->type == JSON_INTEGER) {
> time_t t = json_integer(date);
> - struct tm tm;
> - char s[128];
> -
> - strftime(s, sizeof s, "%Y-%m-%d %H:%M:%S", gmtime_r(&t,
> &tm));
> - printf(" %s", s);
> + char *s = xastrftime(" %Y-%m-%d %H:%M:%S", t, true);
> + fputs(s, stdout);
> + free(s);
> }
>
> comment = shash_find_data(json_object(json), "_comment");
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 409d640..d33eebf 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -1357,13 +1357,9 @@ monitor_vconn(struct vconn *vconn)
> run(retval, "vconn_recv");
>
> if (timestamp) {
> - time_t now = time_wall();
> - struct tm tm;
> - char s[32];
> -
> - strftime(s, sizeof s, "%Y-%m-%d %H:%M:%S: ",
> - gmtime_r(&now, &tm));
> + char *s = xastrftime("%Y-%m-%d %H:%M:%S: ", time_wall(),
> true);
> fputs(s, stderr);
> + free(s);
> }
>
> ofptype_decode(&type, b->data);
> --
> 1.7.2.5
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
