Ticket #4103 (new enhancement)

Opened 6 months ago

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

Change History

comment:1 Changed 6 months ago by andrew_b

Last edited 7 weeks ago by andrew_b (previous) (diff)

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

  • Branch state changed from merged to no branch

Changed 7 weeks ago by and

Changed 7 weeks ago by and

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

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

Changed 6 weeks ago by and

comment:8 Changed 6 weeks ago by andrew_b

I'd prefer break before default: break.

comment:9 Changed 6 weeks ago by and

yes, break looks broad than fallthrough compiler hint

comment:10 Changed 6 weeks ago by and

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

comment:11 Changed 6 weeks ago by andrew_b

Applied with minimal modifications. Thanks!

comment:12 Changed 5 weeks ago by and

tar.c patch is still missing?

Changed 5 weeks ago by and

Changed 5 weeks ago by and

comment:14 Changed 4 weeks 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 weeks 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 weeks ago by and

comment:16 Changed 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks ago by zaytsev

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

comment:22 Changed 3 weeks ago by and

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

Changed 3 weeks ago by and

comment:23 Changed 3 weeks ago by andrew_b

Thanks!

comment:24 Changed 2 weeks 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 2 weeks 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 7 days ago by andrew_b

@zaytsev, ping.

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

Replying to zaytsev:

For me this makes the warning go away.

Let's do it after release.

comment:31 Changed 5 days 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...

Note: See TracTickets for help on using tickets.