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

[Konstantin Kharlamov]

On Пн, Apr 1, 2019 at 15:43, Robert Pluim <rpluim@gmail.com> wrote:
> > > > > >  On Mon, 01 Apr 2019 16:35:50 +0300, Konstantin Kharlamov 
> > > > > > <hi-angel@yandex.ru> 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.