[ovs-dev] [PATCH 10/13] log: Support multiple magic.
Justin Pettit
jpettit at ovn.org
Sun Dec 24 00:11:52 UTC 2017
I forgot to add:
Acked-by: Justin Pettit <jpettit at ovn.org>
> On Dec 23, 2017, at 4:41 PM, Justin Pettit <jpettit at ovn.org> wrote:
>
>
>> 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
>
>
More information about the dev
mailing list