[ovs-dev] [PATCH 10/13] log: Support multiple magic.

[Justin Pettit]

> On Dec 7, 2017, at 5:12 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> diff --git a/ovsdb/log.c b/ovsdb/log.c
> index d25f766474dc..dd641884a2ee 100644
> --- a/ovsdb/log.c
> +++ b/ovsdb/log.c
> 
> +    /* Read the magic from the first log record. */
> +    char header[128];
> +    const char *actual_magic;
> +    if (!fgets(header, sizeof header, stream)) {
> +        if (ferror(stream)) {
> +            error = ovsdb_io_error(errno, "%s: read error", name);
> +            goto error_fclose;
> +        }
> +
> +        /* We need to be able to report what kind of file this is but we can't
> +         * if it's empty and we accept more than one. */
> +        if (strchr(magic, '|')) {
> +            error = ovsdb_error(NULL, "%s: unexpected end of file", name);

An error message indicating that multiple magic values were provided to a new file seems like it might be more helpful.

> void
> ovsdb_log_close(struct ovsdb_log *file)
> {
>     if (file) {
>         ovsdb_error_destroy(file->error);
>         free(file->name);
> +        free(file->magic);

I think this should be added in the first patch of the series.

> static bool
> +parse_header(char *header, const char **magicp,
> +             unsigned long int *length, uint8_t sha1[SHA1_DIGEST_SIZE])

Do you think it's worth pointing out that 'magic' is a pointer into 'header', so it shouldn't be freed?  Also, it looks to be making modifications to 'header'.

> diff --git a/ovsdb/log.h b/ovsdb/log.h
> index cc8469c01e57..4b74ca11ce6a 100644
> --- a/ovsdb/log.h
> +++ b/ovsdb/log.h
> 
> @@ -31,7 +31,7 @@ enum ovsdb_log_open_mode {
>     OVSDB_LOG_CREATE            /* Create or open file, read/write. */
> };
> 
> -#define OVSDB_MAGIC "OVSDB JSON"
> +#define OVSDB_MAGIC "JSON"

It might be worth mentioning that "OVSDB " will be added to the definition.

--Justin