| Message ID | 20240701110257.1052377-2-whitecrowbar@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | Optimize json serialization 2.3 times | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 1 Jul 2024, at 13:02, Grigorii Nazarov wrote: > Signed-off-by: Grigorii Nazarov <whitecrowbar@gmail.com> Hi Grigorii, Thanks for the patch, see some comments below. > --- > v2: fixed title > v4: changed patch number from 2/4 to 1/3 > > lib/ovsdb-data.c | 8 +------- > lib/uuid.h | 1 + > 2 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c > index abb923ad8..defb048d7 100644 > --- a/lib/ovsdb-data.c > +++ b/lib/ovsdb-data.c > @@ -2582,14 +2582,8 @@ char * > ovsdb_data_row_name(const struct uuid *uuid) > { > char *name; > - char *p; > > - name = xasprintf("row"UUID_FMT, UUID_ARGS(uuid)); > - for (p = name; *p != '\0'; p++) { > - if (*p == '-') { > - *p = '_'; > - } > - } > + name = xasprintf(UUID_ROW_FMT, UUID_ARGS(uuid)); > > return name; > } > diff --git a/lib/uuid.h b/lib/uuid.h > index fa49354f6..6a8069f68 100644 > --- a/lib/uuid.h > +++ b/lib/uuid.h > @@ -34,6 +34,7 @@ extern "C" { > */ > #define UUID_LEN 36 > #define UUID_FMT "%08x-%04x-%04x-%04x-%04x%08x" > +#define UUID_ROW_FMT "row%08x_%04x_%04x_%04x_%04x%08x" The “row” addition is rather dbase specific. What about calling it: #define UUID_UNDERSCORE_FMT "%08x_%04x_%04x_%04x_%04x%08x" and add the “row” part in the xasprintf() directly, so: name = xasprintf(“row”UUID_UNDERSCORE_FMT, UUID_ARGS(uuid)); > #define UUID_ARGS(UUID) \ > ((unsigned int) ((UUID)->parts[0])), \ > ((unsigned int) ((UUID)->parts[1] >> 16)), \ > -- > 2.45.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 16 Sep 2024, at 14:12, Grigorii Nazarov wrote: > Hi Eelco, > > Thank you for your reply. > > On 9/6/24 1:09 PM, Eelco Chaudron wrote: >> >> >> On 1 Jul 2024, at 13:02, Grigorii Nazarov wrote: >> >>> Signed-off-by: Grigorii Nazarov <whitecrowbar@gmail.com> >> >> Hi Grigorii, >> >> Thanks for the patch, see some comments below. >> >>> --- >>> v2: fixed title >>> v4: changed patch number from 2/4 to 1/3 >>> >>> lib/ovsdb-data.c | 8 +------- >>> lib/uuid.h | 1 + >>> 2 files changed, 2 insertions(+), 7 deletions(-) >>> >>> diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c >>> index abb923ad8..defb048d7 100644 >>> --- a/lib/ovsdb-data.c >>> +++ b/lib/ovsdb-data.c >>> @@ -2582,14 +2582,8 @@ char * >>> ovsdb_data_row_name(const struct uuid *uuid) >>> { >>> char *name; >>> - char *p; >>> >>> - name = xasprintf("row"UUID_FMT, UUID_ARGS(uuid)); >>> - for (p = name; *p != '\0'; p++) { >>> - if (*p == '-') { >>> - *p = '_'; >>> - } >>> - } >>> + name = xasprintf(UUID_ROW_FMT, UUID_ARGS(uuid)); >>> >>> return name; >>> } >>> diff --git a/lib/uuid.h b/lib/uuid.h >>> index fa49354f6..6a8069f68 100644 >>> --- a/lib/uuid.h >>> +++ b/lib/uuid.h >>> @@ -34,6 +34,7 @@ extern "C" { >>> */ >>> #define UUID_LEN 36 >>> #define UUID_FMT "%08x-%04x-%04x-%04x-%04x%08x" >>> +#define UUID_ROW_FMT "row%08x_%04x_%04x_%04x_%04x%08x" >> >> The “row” addition is rather dbase specific. What about calling it: >> >> #define UUID_UNDERSCORE_FMT "%08x_%04x_%04x_%04x_%04x%08x" >> >> and add the “row” part in the xasprintf() directly, so: >> >> name = xasprintf(“row”UUID_UNDERSCORE_FMT, UUID_ARGS(uuid)); >> >> > Agreed. I'm using it in the next patch, So probably the right decision would be to move UUID_ROW_FMT to lib/ovsdb-data.c file instead. It's not otherwise needed even partially in lib/uuid.h. If you use my approach above, you could keep UUID_UNDERSCORE_FMT in uuid.h >>> #define UUID_ARGS(UUID) \ >>> ((unsigned int) ((UUID)->parts[0])), \ >>> ((unsigned int) ((UUID)->parts[1] >> 16)), \ >>> -- >>> 2.45.2 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > -- > Best, > Grigorii
diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c index abb923ad8..defb048d7 100644 --- a/lib/ovsdb-data.c +++ b/lib/ovsdb-data.c @@ -2582,14 +2582,8 @@ char * ovsdb_data_row_name(const struct uuid *uuid) { char *name; - char *p; - name = xasprintf("row"UUID_FMT, UUID_ARGS(uuid)); - for (p = name; *p != '\0'; p++) { - if (*p == '-') { - *p = '_'; - } - } + name = xasprintf(UUID_ROW_FMT, UUID_ARGS(uuid)); return name; } diff --git a/lib/uuid.h b/lib/uuid.h index fa49354f6..6a8069f68 100644 --- a/lib/uuid.h +++ b/lib/uuid.h @@ -34,6 +34,7 @@ extern "C" { */ #define UUID_LEN 36 #define UUID_FMT "%08x-%04x-%04x-%04x-%04x%08x" +#define UUID_ROW_FMT "row%08x_%04x_%04x_%04x_%04x%08x" #define UUID_ARGS(UUID) \ ((unsigned int) ((UUID)->parts[0])), \ ((unsigned int) ((UUID)->parts[1] >> 16)), \
Signed-off-by: Grigorii Nazarov <whitecrowbar@gmail.com> --- v2: fixed title v4: changed patch number from 2/4 to 1/3 lib/ovsdb-data.c | 8 +------- lib/uuid.h | 1 + 2 files changed, 2 insertions(+), 7 deletions(-)