Ticket #4103 (closed enhancement: fixed)

Opened 9 months ago

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

Change History

comment:1 Changed 9 months ago by andrew_b

Last edited 5 months ago by andrew_b (previous) (diff)

comment:2 Changed 7 months 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 6 months 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 6 months ago by andrew_b

  • Branch state changed from merged to no branch

Changed 5 months ago by and

Changed 5 months ago by and

comment:6 Changed 5 months 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 5 months ago by and

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

Changed 4 months ago by and

comment:8 Changed 4 months ago by andrew_b

I'd prefer break before default: break.

comment:9 Changed 4 months ago by and

yes, break looks broad than fallthrough compiler hint

comment:10 Changed 4 months ago by and

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

comment:11 Changed 4 months ago by andrew_b

Applied with minimal modifications. Thanks!

comment:12 Changed 4 months ago by and

tar.c patch is still missing?

Changed 4 months ago by and

comment:14 Changed 4 months 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 months ago by andrew_b

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

comment:16 Changed 4 months 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 months 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 months 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 months 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 months 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 months ago by zaytsev

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

comment:22 Changed 4 months ago by and

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

Changed 4 months ago by and

comment:23 Changed 4 months ago by andrew_b

Thanks!

comment:24 Changed 4 months 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 months 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 3 months ago by andrew_b

@zaytsev, ping.

comment:27 follow-up: ↓ 30 Changed 3 months 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 3 months 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 3 months 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 3 months ago by andrew_b

Replying to zaytsev:

For me this makes the warning go away.

Let's do it after release.

comment:31 Changed 3 months 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 3 months 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.