bug-gnu-emacs
[Top][All Lists]
Advanced

[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:35:50 +0300



On Пн, Apr 1, 2019 at 15:27, Robert Pluim <address@hidden> wrote:
On Mon, 01 Apr 2019 07:37:23 +0300, Eli Zaretskii <address@hidden> said:

    >> From: Konstantin Kharlamov <address@hidden> Date: Mon, 1
    >> Apr 2019 01:37:38 +0300
    >>
    >> These are mostly fixes of some of LGTM warnings
>> https://lgtm.com/projects/g/emacs-mirror/emacs/alerts/?mode=tree
    >>
    >> Except the second patch, where I initially wanted to fix one
    >> warning, and as part of it I had to constify a variable to see
    >> that it is indeed immutable. And then I figured I could search
    >> through the code and find more similar places, where variables
    >> weren't marked as const. I like this cleanup because it is
    >> simple and trivially testable (i.e. if it compiles, then it's
    >> fine). FTR: there's still lots of opportunities for
    >> constification, I just stopped at some point.

    Eli> Thanks.

    Eli> I think the general policy is not to fix those except when
    Eli> making other changes in the same function, but I will let
    Eli> others comment.

Iʼd prefer it if the effort went to determining if eg the alert for
'type = 2' below was correct or not, proving the constness of
variables is what we have a compiler for.

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

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

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







reply via email to

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