Re: Yet another small patch - reorderbuffer.c:1099

From: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Yet another small patch - reorderbuffer.c:1099
Date: 2016-04-06 10:07:33
Message-ID: 20160406130733.686c2764@fujitsu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> > IMO the code is wrong. There should be a single block comment
> > saying something like "Remove the node from its containing list.
> > In the FOO case, the list corresponds to BAR and therefore we
> > delete it because BAZ. In the QUUX case the list is PLUGH and we
> > delete in because THUD." Then a single line dlist_delete(...)
> > follows.
> >
> > The current arrangement looks bizantine to me, for no reason. If we
> > think that one of the two branches might do something additional to
> > the list deletion, surely that will be in a separate stanza with
> > its own comment; and if we ever want to remove the list deletion
> > from one of the two cases (something that strikes me as unlikely)
> > then we will need to fix the comment, too.
> +1 to everything here except for the way byzantine is spelled.

+1 and yet another patch.

Best regards,
Aleksander Alekseev

Attachment Content-Type Size
reorderbuffer-v2.diff text/x-patch 1.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-04-06 10:15:00 Re: Yet another small patch - reorderbuffer.c:1099
Previous Message Andres Freund 2016-04-06 09:52:28 Re: Move PinBuffer and UnpinBuffer to atomics