| Message ID | 1565334707-29894-1-git-send-email-damjan.skvarc@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v2,ovn] logical-fields: fix memory leak while initializing ovnfield_by_name | expand |
Bleep bloop. Greetings Damijan Skvarc, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: fatal: sha1 information is lacking or useless (lib/logical-fields.c). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 logical-fields: fix memory leak while initializing ovnfield_by_name The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
On 09.08.2019 10:11, Damijan Skvarc wrote: > According to comments I have: > - renamed function name to be "paired" with ovn_destroy_ovnfields > - replaced 0/1 bool values with false/true > > Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com> > --- > lib/logical-fields.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/lib/logical-fields.c b/lib/logical-fields.c > index fdc78cf..8a0ef62 100644 > --- a/lib/logical-fields.c > +++ b/lib/logical-fields.c > @@ -58,18 +58,18 @@ add_ct_bit(const char *name, int index, struct shash *symtab) > } > > static void > -init_ovnfield_by_name(void) > +ovn_init_ovnfields(void) > { > - static bool initialized = 0; > + static bool initialized = false; There is a special mechanism 'ovsthread_once' for this kind of things. Can we use it instead? Best regards, Ilya Maximets. > > - if (0 == initialized) { > + if (!initialized) { > shash_init(&ovnfield_by_name); > for (int i = 0; i < OVN_FIELD_N_IDS; i++) { > const struct ovn_field *of = &ovn_fields[i]; > ovs_assert(of->id == i); /* Fields must be in the enum order. */ > shash_add_once(&ovnfield_by_name, of->name, of); > } > - initialized = 1; > + initialized = true; > } > } > > @@ -234,7 +234,7 @@ ovn_init_symtab(struct shash *symtab) > expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false); > expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false); > > - init_ovnfield_by_name(); > + ovn_init_ovnfields(); > > expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU); > } >
On 09.08.2019 10:11, Damijan Skvarc wrote: > According to comments I have: > - renamed function name to be "paired" with ovn_destroy_ovnfields > - replaced 0/1 bool values with false/true > > Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com> This patch named "fix memory leak ...", but I don't see any leaks fixed in the code. It seems that you've sent diff between v1 and v2 instead of the patch itself. If so, you need to squash this into your v1 patch and send as v3. Best regards, Ilya Maximets.
diff --git a/lib/logical-fields.c b/lib/logical-fields.c index fdc78cf..8a0ef62 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -58,18 +58,18 @@ add_ct_bit(const char *name, int index, struct shash *symtab) } static void -init_ovnfield_by_name(void) +ovn_init_ovnfields(void) { - static bool initialized = 0; + static bool initialized = false; - if (0 == initialized) { + if (!initialized) { shash_init(&ovnfield_by_name); for (int i = 0; i < OVN_FIELD_N_IDS; i++) { const struct ovn_field *of = &ovn_fields[i]; ovs_assert(of->id == i); /* Fields must be in the enum order. */ shash_add_once(&ovnfield_by_name, of->name, of); } - initialized = 1; + initialized = true; } } @@ -234,7 +234,7 @@ ovn_init_symtab(struct shash *symtab) expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false); expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false); - init_ovnfield_by_name(); + ovn_init_ovnfields(); expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU); }
According to comments I have: - renamed function name to be "paired" with ovn_destroy_ovnfields - replaced 0/1 bool values with false/true Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com> --- lib/logical-fields.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)