That's how I roll. Thanks, applied to master.
On Mon, May 06, 2013 at 01:02:08PM -0700, Andy Zhou wrote:
> 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
