[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#35062: [PATCH 0/4] Trivial code cleanups

From: Konstantin Kharlamov
Subject: bug#35062: [PATCH 0/4] Trivial code cleanups
Date: Mon, 01 Apr 2019 16:51:47 +0300

On Пн, Apr 1, 2019 at 15:43, Robert Pluim <address@hidden> wrote:
On Mon, 01 Apr 2019 16:35:50 +0300, Konstantin Kharlamov <address@hidden> said:
    >> xterm.c:5346
    >> if (XSCROLL_BAR (bar)->x_window == window_id && FRAME_X_DISPLAY
    >> (XFRAME (frame)) == display && (type = 2 || (type == 1 &&
    >> XSCROLL_BAR (bar)->horizontal) || (type == 0 && !XSCROLL_BAR
    >> (bar)->horizontal))) return XSCROLL_BAR (bar);
    >> Robert

    Konstantin> Well, not everything at once! :) I afraid that if I
    Konstantin> fix lots of warnings in one patch-set, it may get
    Konstantin> stuck in review because of the amount of changes;
    Konstantin> besides it's easier for my sanity to send small
    Konstantin> patchsets because mailing-list based projects in
    Konstantin> general tend not to accept patches too quickly.

Thatʼs why you concentrate on things that look like errors :-)

    Konstantin> Also note: the constness here is not for compiler but
    Konstantin> for developers.

I donʼt see how developers are particularly helped in this particular
case, but then again they're not harmed either.

For illustration you can take a look at patch 3/4 here. There, I constify min_rows and min_cols; and afterwards I remove a paragraph that compares them to a number that wasn't assigned there. This allows to not look through the code before the comparison because it's immediately obvious: these variables are never changed.

This is true for reading the code as well, especially when you're debugging a problem: you may often wonder "okay, when was the last time that variable changed, could it be invalid here?". Then, if it's "const" you immediately know the answer, whereas otherwise you have to search through all usages of that variable to see when was the last time it changed.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]