Ticket #4103 (closed enhancement: fixed)

Opened 4 years ago

Last modified 4 years ago

Prepare for release mc-4.8.26

Reported by: zaytsev Owned by:
Priority: major Milestone: 4.8.26
Component: adm Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset:

Description


Attachments

mc-4103-1-panel.c-fix-shadow-warning.patch (1.3 KB) - added by and 4 years ago.
mc-4103-2-generic-detection-of-attribute-fallthrough.patch (1.5 KB) - added by and 4 years ago.
mc-4103-3-remove-unused-m4-macro-file.patch (9.5 KB) - added by and 4 years ago.
mc-4103-4-hotlist.c-fix-fallthrough-annotation-warning.patch (797 bytes) - added by and 4 years ago.
mc-4103-fix-fallthrough-warnings.patch (4.7 KB) - added by and 4 years ago.
mc-4103-cid-extfs.c-fix-uninitialized-scalar-variable.patch (1.5 KB) - added by and 4 years ago.
mc-4103-cid-extfs.c-fix-uninitialized-scalar-variable.2.patch (1.5 KB) - added by and 4 years ago.
mc-4103-cid-extfs.c-fix-uninitialized-scalar-variable.3.patch (1.5 KB) - added by and 4 years ago.
mc-4103-cid-find.c-fix-uninitialized-scalar-variable.patch (875 bytes) - added by and 4 years ago.
mc-4103-cid-fish.c-fix-uninitialized-scalar-variable.patch (1.2 KB) - added by and 4 years ago.
mc-4103-cid-tar.c-fix-uninitialized-scalar-variable.patch (954 bytes) - added by and 4 years ago.
mc-4103-cid-widget-common.c-fix-uninitialized-scalar-variable.patch (997 bytes) - added by and 4 years ago.
mc-4103-cid-background.c-fix.resource-leak.patch (724 bytes) - added by and 4 years ago.
mc-4103-cid-utilunix.c-fix.resource-leak.patch (1.2 KB) - added by and 4 years ago.
mc-4103-cid-cons.handler.c-fix.resource-leak.patch (3.5 KB) - added by and 4 years ago.
mc-4103-cid-command.c-fix-out-of-bounds-read.patch (839 bytes) - added by and 4 years ago.
mc-4103-cid-cpio.c-fix-unintended-sign-extension.patch (1.3 KB) - added by and 4 years ago.
mc-4103-cid-editcmd.c-fix-dereference-before-null-check.patch (1.2 KB) - added by and 4 years ago.
mc-4103-cid-growbuf.c-fix-dereference-before-null-check.patch (1.6 KB) - added by and 4 years ago.
mc-4103-cid-syntax.c-fix-logically-dead-code.patch (950 bytes) - added by and 4 years ago.
mc-4103-cid-util.c-fix-logically-dead-code.patch (810 bytes) - added by and 4 years ago.
mc-4103-cid-coord_cache.c-fix-wrong-sizeof-argument.patch (956 bytes) - added by and 4 years ago.
mc-4103-cid-editcmd.c-fix-unchecked-return-value-warning.patch (875 bytes) - added by and 4 years ago.
mc-4103-cid-editdraw.c-fix-copy-paste-error.patch (794 bytes) - added by and 4 years ago.
mc-4103-cid-ioblksize.h-fix-big-parameter-passed-by-value-warning.patch (1.3 KB) - added by and 4 years ago.
mc-4103-cid-layout.c-fix-dereference-after-null-check-warning.patch (904 bytes) - added by and 4 years ago.
mc-4103-cid-treestore.c-fix-copy-into-fixed-size-buffer-warning.patch (1.3 KB) - added by and 4 years ago.
mc-4103-fix-tests-warning.patch (877 bytes) - added by and 4 years ago.

Change History

comment:1 Changed 4 years ago by andrew_b

Last edited 4 years ago by andrew_b (previous) (diff)

comment:2 Changed 4 years ago by zaytsev

Latest lintian report:

W groff-message usr/share/man/ru/man1/mc.1.gz 3486: missing \?
W groff-message usr/share/man/ru/man1/mc.1.gz 3487: warning: macro '\' not defined
W national-encoding usr/share/mc/syntax/c.syntax
W national-encoding usr/share/mc/syntax/cs.syntax
W national-encoding usr/share/mc/syntax/cuda.syntax
W national-encoding usr/share/mc/syntax/cxx.syntax
W national-encoding usr/share/mc/syntax/d.syntax
W national-encoding usr/share/mc/syntax/haskell.syntax
W national-encoding usr/share/mc/syntax/idl.syntax
W national-encoding usr/share/mc/syntax/nemerle.syntax
W national-encoding usr/share/mc/syntax/opencl.syntax
W national-encoding usr/share/mc/syntax/osl.syntax
W national-encoding usr/share/mc/syntax/slang.syntax
W national-encoding usr/share/mc/syntax/swig.syntax
W national-encoding usr/share/mc/syntax/yxx.syntax
I acute-accent-in-manual-page usr/share/man/es/man1/mc.1.gz:2703
I acute-accent-in-manual-page usr/share/man/es/man1/mc.1.gz:2704
I acute-accent-in-manual-page usr/share/man/hu/man1/mc.1.gz:2328
I acute-accent-in-manual-page usr/share/man/hu/man1/mc.1.gz:2329
I acute-accent-in-manual-page usr/share/man/it/man1/mc.1.gz:2360
I acute-accent-in-manual-page usr/share/man/man1/mc.1.gz:2712
I acute-accent-in-manual-page usr/share/man/man1/mc.1.gz:4081
I acute-accent-in-manual-page usr/share/man/pl/man1/mc.1.gz:1987
I acute-accent-in-manual-page usr/share/man/ru/man1/mc.1.gz:3045
I acute-accent-in-manual-page usr/share/man/ru/man1/mc.1.gz:4536
I acute-accent-in-manual-page usr/share/man/sr/man1/mc.1.gz:1193
I acute-accent-in-manual-page usr/share/man/sr/man1/mc.1.gz:2352
I acute-accent-in-manual-page usr/share/man/sr/man1/mc.1.gz:406

comment:4 Changed 4 years ago by andrew_b

  • Branch state changed from no branch to merged

Merged to master: [165ec3844474485619c53d9df96462eebf7bef79].

git log --pretty=oneline eeb14d3f4..165ec3844

comment:5 Changed 4 years ago by andrew_b

  • Branch state changed from merged to no branch

Changed 4 years ago by and

Changed 4 years ago by and

comment:6 Changed 4 years ago by andrew_b

mc-4103-1-panel.c-fix-shadow-warning.patch​ squashed with related commit.
mc-4103-4-hotlist.c-fix-fallthrough-annotation-warning.patch​ applied.
Thanks!

As for
mc-4103-2-generic-detection-of-attribute-fallthrough.patch​
mc-4103-3-remove-unused-m4-macro-file.patch​
not only gcc >= 5, clang >= 2.9 can be used to build mc on various (including proprietary) OSes. I think we should keep compatibility with those systems.

comment:7 Changed 4 years ago by and

hoestly there will be no compiler out there with __attribute__((fallthrough)) and without __has_attribute macro support.

Changed 4 years ago by and

comment:8 Changed 4 years ago by andrew_b

I'd prefer break before default: break.

comment:9 Changed 4 years ago by and

yes, break looks broad than fallthrough compiler hint

comment:10 Changed 4 years ago by and

all cid patches reduce High Impact Outstanding Coverity counter
(ignore same extfs patch attachment mistake)

comment:11 Changed 4 years ago by andrew_b

Applied with minimal modifications. Thanks!

comment:12 Changed 4 years ago by and

tar.c patch is still missing?

Changed 4 years ago by and

Changed 4 years ago by and

comment:14 Changed 4 years ago by and

Hi Andrew, any comment about out-of-bounds read patch?

When do_cd_command() get "cd" input, cmd will replace by "cd ".

First condition test checking cmd[0] for \0 is always false (cmd[0] will be always 'c')
Hence next condition test DIR_IS_DOTDOT (cmd + operand_pos) will out-of-bounds read access,
for input without cd path information.

Fixing first condition test to restore original goal to catch 'cd' without path information to not trigger second condition test

comment:15 Changed 4 years ago by andrew_b

I think the best way is make some refactoring: do_cd_command() should get path only without leading "cd".

Changed 4 years ago by and

comment:16 Changed 4 years ago by andrew_b

  • mc-4103-cid-coord_cache.c-fix-wrong-sizeof-argument.patch​

Coverity is fine, but you should not trust it unconditionally. This code is correct. cache->cache is array of pointers, sizeof (*cache->cache) is size of pointer and one pointer is inserted into array here. I think we should reimplement cache using GPtrArray like it was before #1585. Let's do it after soon MC release.


  • mc-4103-cid-ioblksize.h-fix-big-parameter-passed-by-value-warning.patch

Function is inline, so in fact entire structure is not copied.


  • mc-4103-cid-layout.c-fix-dereference-after-null-check-warning.patch

mignight_dlg is not NULL when create_panel() is called. mignight_dlg is already dereferenced in line 1130. Test mignight_dlg == NULL in line 1196 should be removed instead of adding it in line 1213.


  • mc-4103-cid-treestore.c-fix-copy-into-fixed-size-buffer-warning.patch

g_strlcpy() is preferred to use here and below in line 297.

comment:17 Changed 4 years ago by andrew_b

I think we should take a pause in cleaning up until next release.

Branch 4103_cleanup is rebased on top of master: [afa24a410357f588f7d7f7b334dfa08a324d3aba]. Let's test it before merge.

@zaytsev, Yuri when you can spend some time to make release?

comment:18 follow-up: ↓ 20 Changed 4 years ago by zaytsev

Hi Andrew,

Thanks for asking, I was about to write you an email today to ask the same... My situation is as follows: thanks to COVID and pressure at $DAYJOB I'm not gonna get a leave this time around, so I'll be working over the "holidays". However, at least there will be less people around, so I will be able to make time for a release as usual.

What are your plans regarding the release? I could try to cut an RC on the weekend, or at least begin working on it... Do you need to merge or finish anything before release?

--Yury

comment:19 Changed 4 years ago by zaytsev

For example I've just noticed a Swift PR - would you take it before the release?

comment:20 in reply to: ↑ 18 Changed 4 years ago by andrew_b

Replying to zaytsev:

What are your plans regarding the release? I could try to cut an RC on the weekend, or at least begin working on it... Do you need to merge or finish anything before release?

Yes, I need merge 4103_cleanup before release, but I need few days to use it as working version.

Swift PR - would you take it before the release

Ok, I'll commit it to master today.

comment:21 Changed 4 years ago by zaytsev

Alright, just let me know - I could also start on Monday / Tuesday next week.

comment:22 Changed 4 years ago by and

Thanks Andrew,
I uploaded latest 4103_cleanup state to coverity
and a single warning fix lately introduces only.

Changed 4 years ago by and

comment:23 Changed 4 years ago by andrew_b

Thanks!

comment:24 Changed 4 years ago by zaytsev

Hi Andrew,

So what do you think of the state of master? Shall I attempt at an RC?

All the best,
Yury

comment:25 Changed 4 years ago by andrew_b

  • Branch state changed from no branch to merged

I've merged cleanup to the master: [485ea75cbe54ec201291d67011723ff2701e0a6c].

git log --pretty=oneline 18a300e64..485ea75cb

Master is ready for RC now.

comment:26 Changed 4 years ago by andrew_b

@zaytsev, ping.

comment:27 follow-up: ↓ 30 Changed 4 years ago by zaytsev

I get the following warning with gcc 10.2:

[zaytsev@localhost dist]$ gcc -v
...
gcc version 10.2.1 20201125 (Red Hat 10.2.1-9) (GCC) 
../../../lib/tty/color-internal.c: In function ‘tty_color_get_index_by_name’:
../../../lib/tty/color-internal.c:156:19: warning: ‘h[0]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  156 |             i = (h[0] << 20) | (h[1] << 16) | (h[2] << 12) | (h[3] << 8) | (h[4] << 4) | h[5];
      |                  ~^~~
../../../lib/tty/color-internal.c:156:34: warning: ‘h[1]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  156 |             i = (h[0] << 20) | (h[1] << 16) | (h[2] << 12) | (h[3] << 8) | (h[4] << 4) | h[5];
      |                                 ~^~~
../../../lib/tty/color-internal.c:156:49: warning: ‘h[2]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  156 |             i = (h[0] << 20) | (h[1] << 16) | (h[2] << 12) | (h[3] << 8) | (h[4] << 4) | h[5];
      |                                                ~^~~

Apparently GCC cannot understand, that len > 0 because we exit if len != 3 && len != 6. Do you think it makes sense to commit the following? For me this makes the warning go away.

diff --git a/lib/tty/color-internal.c b/lib/tty/color-internal.c
index d94319808..e6d67bcbc 100644
--- a/lib/tty/color-internal.c
+++ b/lib/tty/color-internal.c
@@ -152,7 +152,7 @@ parse_256_or_true_color_name (const char *color_name)
 
         if (i == 3)
             i = (h[0] << 20) | (h[0] << 16) | (h[1] << 12) | (h[1] << 8) | (h[2] << 4) | h[2];
-        else
+        else if (i == 6)
             i = (h[0] << 20) | (h[1] << 16) | (h[2] << 12) | (h[3] << 8) | (h[4] << 4) | h[5];
         return (1 << 24) | i;
     }

comment:28 Changed 4 years ago by zaytsev

Bad news - Travis CI is basically bust - https://www.jeffgeerling.com/blog/2020/travis-cis-new-pricing-plan-threw-wrench-my-open-source-works . I have repaired it for now, and hopefully this will buy us some time, but I would have to migrate from Travis CI to GitHub Actions now...

comment:29 Changed 4 years ago by zaytsev

prepare repository for release
download PO-translations from Transifex
store translations in git repo
download the hint translations from Transifex
store translations in git repo

done

create new NEWS wiki page

NEWS-4.8.27

add content of current NEWS wiki page to the doc/NEWS file in git repo
new version in Trac
new milestone in Trac

done

Sent a mail around with the RC tarball.

comment:30 in reply to: ↑ 27 Changed 4 years ago by andrew_b

Replying to zaytsev:

For me this makes the warning go away.

Let's do it after release.

comment:31 Changed 4 years ago by zaytsev

Let's do it after release.

Yeah, maybe even better would be to convert it into a switch case with a fallthrough assert...

comment:32 Changed 4 years ago by zaytsev

  • Status changed from new to closed
  • Resolution set to fixed

create new tag in git
create tar.(bz2|xz) package files
make checksums for archives

done

upload source packages and checksums to the special upload area

skipped

developers should download tarballs, verify checksums, compile and install locally

done

upload source packages and checksums
run command: ssh midnightcommander@…
update Wiki start page with latest release number
write an announcement: list user visible changes (bugs and features)

done

create new ticket (type=task, component=adm) for the next release

#4179

close ticket for release
close current milestone

done

Note: See TracTickets for help on using tickets.