[ovs-dev,6/7] raft.c: cmd->eid should always be non-null.

Message ID 1554859282-15144-6-git-send-email-hzhou8@ebay.com
State New
Headers show
Series
  • [ovs-dev,1/7] ovsdb raft: Sync commit index to followers without delay.
Related show

Commit Message

Han Zhou April 10, 2019, 1:21 a.m.
From: Han Zhou <hzhou8@ebay.com>

raft_command's eid should always be non-null in all 3 cases. Fix the
comment, and also replace if condition with assert.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
---
 ovsdb/raft.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Ben Pfaff April 12, 2019, 4:44 p.m. | #1
On Tue, Apr 09, 2019 at 06:21:21PM -0700, Han Zhou wrote:
> From: Han Zhou <hzhou8@ebay.com>
> 
> raft_command's eid should always be non-null in all 3 cases. Fix the
> comment, and also replace if condition with assert.
> 
> Signed-off-by: Han Zhou <hzhou8@ebay.com>
> ---
>  ovsdb/raft.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/ovsdb/raft.c b/ovsdb/raft.c
> index abcc9c0..32dcea6 100644
> --- a/ovsdb/raft.c
> +++ b/ovsdb/raft.c
> @@ -112,7 +112,7 @@ struct raft_command {
>      /* Case 1 only. */
>      uint64_t index;             /* Index in log (0 if being relayed). */
>  
> -    /* Cases 2 and 3. */
> +    /* Cases 1, 2 and 3. */
>      struct uuid eid;            /* Entry ID of result. */
>  
>      /* Case 2 only. */

Thanks for the correction.  Since there's already a section for "all
cases", I'd suggest moving the eid member there.

Thanks,

Ben.
Han Zhou April 12, 2019, 5:04 p.m. | #2
On Fri, Apr 12, 2019 at 9:44 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Tue, Apr 09, 2019 at 06:21:21PM -0700, Han Zhou wrote:
> > From: Han Zhou <hzhou8@ebay.com>
> >
> > raft_command's eid should always be non-null in all 3 cases. Fix the
> > comment, and also replace if condition with assert.
> >
> > Signed-off-by: Han Zhou <hzhou8@ebay.com>
> > ---
> >  ovsdb/raft.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/ovsdb/raft.c b/ovsdb/raft.c
> > index abcc9c0..32dcea6 100644
> > --- a/ovsdb/raft.c
> > +++ b/ovsdb/raft.c
> > @@ -112,7 +112,7 @@ struct raft_command {
> >      /* Case 1 only. */
> >      uint64_t index;             /* Index in log (0 if being relayed).
*/
> >
> > -    /* Cases 2 and 3. */
> > +    /* Cases 1, 2 and 3. */
> >      struct uuid eid;            /* Entry ID of result. */
> >
> >      /* Case 2 only. */
>
> Thanks for the correction.  Since there's already a section for "all
> cases", I'd suggest moving the eid member there.
>
OK.

Patch

diff --git a/ovsdb/raft.c b/ovsdb/raft.c
index abcc9c0..32dcea6 100644
--- a/ovsdb/raft.c
+++ b/ovsdb/raft.c
@@ -112,7 +112,7 @@  struct raft_command {
     /* Case 1 only. */
     uint64_t index;             /* Index in log (0 if being relayed). */
 
-    /* Cases 2 and 3. */
+    /* Cases 1, 2 and 3. */
     struct uuid eid;            /* Entry ID of result. */
 
     /* Case 2 only. */
@@ -1974,9 +1974,8 @@  raft_command_initiate(struct raft *raft,
     }
 
     struct raft_command *cmd = raft_command_create_incomplete(raft, index);
-    if (eid) {
-        cmd->eid = *eid;
-    }
+    ovs_assert(eid);
+    cmd->eid = *eid;
 
     raft_waiter_create(raft, RAFT_W_ENTRY, true)->entry.index = cmd->index;