[ovs-dev,v2,ovn] logical-fields: fix memory leak while initializing ovnfield_by_name
diff mbox series

Message ID 1565334707-29894-1-git-send-email-damjan.skvarc@gmail.com
State New
Headers show
Series
  • [ovs-dev,v2,ovn] logical-fields: fix memory leak while initializing ovnfield_by_name
Related show

Commit Message

Damijan Skvarc Aug. 9, 2019, 7:11 a.m. UTC
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(-)

Comments

0-day Robot Aug. 9, 2019, 7:57 a.m. UTC | #1
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
Ilya Maximets Aug. 9, 2019, 9:52 a.m. UTC | #2
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);
>  }
>
Ilya Maximets Aug. 9, 2019, 10 a.m. UTC | #3
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.

Patch
diff mbox series

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);
 }