git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] notes.c: fix off-by-one error when decreasing notes fanout

From
Johan Herland <johan@herland.net>
Date
Feb 5, 2020, 17:16 UTC
Message-ID
<CALKQrgcbo-giO=5c=+=KtVxFth0WOB5j+M3Lw26PpLFd+Pqpng@mail.gmail.com>
In-Reply-To
<20200204010523.GR4113372@camp.crustytoothpaste.net>

On Tue, Feb 4, 2020 at 2:05 AM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 5 quoted lines
> On 2020-02-03 at 21:04:45, Johan Herland wrote:
> > always follow the ->next pointer from the last non-note we wrote.
> > (This part is was caught by an existing test in t3304.)
>
> I think you have "is was" here.  You probably want one or the other.
Will fix.
Show 6 quoted lines
> > Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > Cc: Brian M. Carlson <sandals@crustytoothpaste.net>
>
> I generally write my name in lower case, but I think typically we prefer
> to omit Cc lines in patches (unlike LKML), so it may just be better to
> remove these lines.
Ack. Will remove.
Show 5 quoted lines
> > Signed-off-by: Johan Herland <johan@herland.net>
>
> Patch 1 looked good to me.  Your explanation here makes sense, but I
> must admit that I still don't understand this code, so I can't give an
> outright approval.  I do appreciate that it comes with a test, though.

Thanks for having a look. I must admit it's hard to get back into this code even though I originally wrote it. I've been trying to prepare a patch #3 which decreases the fanout more aggressively on notes removal (having a fanout of 1 in a tree of 17 notes is bit ridiculous, IMHO), but I'm not yet able to figure something out that behaves in a stable manner. (I find scenarios where removing a note will switch fanout from 1 to 0, but removing another note will then switch fanout from 0 back to 1, and so on, and the current notes code does not have these problems, AFAICS.)

> I haven't tested, but I expect this series will make Dscho's patch
> unnecessary, so I'll drop it in my reroll unless one of you tells me to
> keep it.

Yes, my patch includes Dscho's change. I don't particularly care whichever lands first, and I can easily rebase on top of yours.

...Johan
-- 
Johan Herland, <johan@herland.net>
www.herland.net
Previous: brian m. carlson
Message 4 of 4 in “t3305: check notes fanout more carefully and robustly”
  1. 1/2 t3305: check notes fanout more carefully and robustlyJohan Herland, Feb 3, 2020
  2. 2/2 notes.c: fix off-by-one error when decreasing notes fanoutJohan Herland, Feb 3, 2020
  3. brian m. carlsonFeb 4, 2020
  4. Johan HerlandFeb 5, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.