Hi,

On Mon, Jan 8, 2018 at 3:45 PM, Yasushi SHOJI <yasushi.sh...@gmail.com> wrote:
> Hi all,
>
> Thank you guys for insightful help.  I just read the code and now I understand
> what you guys are saying.  Yeah, I can say the fix is "spot on".
>
> But, to be honest, it's hard to see why you need "if (p)" at  the first 
> glance.
> So, how about this fix, instead?

+    for (p = list, i = 0; i < cnt; i++, p = p->next) {

Here "i" can reach "cnt - 1" at most, so ...

+        if (i == cnt) {
+            /* clean-up unused elements if any */
+            free_commit_list(p->next);
+            p->next = NULL;
+        }

... "i == cnt" is always false above. I think it should be "i == cnt - 1".

And with your code one can wonder why the cleanup is part of the loop.

> I also found a bug in show_list().  I'm attaching a patch as well.

Could you send it inline as explained in Documentation/SubmittingPatches?

Thanks,
Christian.

Reply via email to