Ticket #3693 (new task)

Opened 5 months ago

Last modified 3 weeks ago

Prepare for release mc-4.8.19

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

Description


Attachments

mc-3641-cleanup-Wformat-signedness-common_c.patch (1.1 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-editdraw_c.patch (3.4 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-file_c.patch (1.2 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-find_c.patch (2.6 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-ftpfs_c.patch (1.1 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-hex_c.patch (1.2 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-info_c.patch (3.1 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-midnight_c.patch (1.7 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-path_c.patch (1.0 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wformat-signedness-serialize_c.patch (4.5 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-Wunsafe-loop-optimizations.patch (1.1 KB) - added by andrew_b 5 months ago.
mc-3641-cleanup-no-attribute-noreturn-utilunix.c.patch (765 bytes) - added by andrew_b 5 months ago.
mc-3641-cleanup-no-attribute-noreturn-utilvfs.c.patch (1005 bytes) - added by andrew_b 5 months ago.
mc-3641-cleanup-unreachable-code-warning-utilunix_c.patch (930 bytes) - added by andrew_b 5 months ago.
3693-Hex-patterns-fix-manual-page.patch (1.2 KB) - added by mooffie 4 months ago.
3693-mc_search_run-document-the-return-value.patch (1.1 KB) - added by mooffie 3 months ago.
This documentation is needed in order to understand the bug(s) in #3720.
3693-mc_search__cond_struct_new_regex_ci_str-add-documentation.patch (1.2 KB) - added by mooffie 3 months ago.
(This documentation is needed to better understand the patch I'll have for #3589.)
3693-mcedit-syntax.syntax-highlight-comments-preceded-by-spaces.patch (878 bytes) - added by mooffie 3 months ago.
mc-3693-dialogs.c-Cleanup-maybe-uninitialized-compiler-warni.patch (973 bytes) - added by and 3 months ago.
mc-3693-edit.c-Cleanup-unsafe-loop-optimizations-compiler-wa.patch (1.3 KB) - added by and 3 months ago.
mc-3693-editcmd.c-Cleanup-comma-compiler-warning.patch (1.5 KB) - added by and 3 months ago.
mc-3693-editcmd_dialogs.c-Cleanup-maybe-uninitialized-compil.patch (1.6 KB) - added by and 3 months ago.
mc-3693-execute.c-Cleanup-maybe-uninitialized-compiler-warni.patch (1.1 KB) - added by and 3 months ago.
mc-3693-file.c-Cleanup-maybe-uninitialized-compiler-warning.patch (1.0 KB) - added by and 3 months ago.
mc-3693-internal.c-Cleanup-format-warning.patch (981 bytes) - added by and 3 months ago.
mc-3693-lib.c-Cleanup-comma-compiler-warning.patch (1.2 KB) - added by and 3 months ago.
mc-3693-main.c-Cleanup-maybe-uninitialized-compiler-warning.patch (982 bytes) - added by and 3 months ago.
mc-3693-panel.c-Cleanup-comma-compiler-warning.patch (877 bytes) - added by and 3 months ago.
mc-3693-search.c-Cleanup-maybe-uninitialized-compiler-warnin.patch (943 bytes) - added by and 3 months ago.
mc-3693-syntax.c-Cleanup-unsafe-loop-optimizations-compiler-.patch (1.5 KB) - added by and 3 months ago.
mc-3641-3571-Fix-aborts-of-MSG_MOUSE_DOWN.patch (1.0 KB) - added by mooffie 3 weeks ago.

Change History

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

Changed 5 months ago by andrew_b

comment:1 Changed 5 months ago by andrew_b

Branch: 3693_cleanup.
Initial changeset:c0c14690d8bd81ca980af265230224ead9891b17

mc-3641-cleanup-Wformat-signedness-common_c.patch​: applied.
mc-3641-cleanup-Wformat-signedness-editdraw_c.patch​​: applied.
mc-3641-cleanup-Wformat-signedness-file_c.patch​​: applied.
mc-3641-cleanup-Wformat-signedness-find_c.patch​: applied.
mc-3641-cleanup-Wformat-signedness-ftpfs_c.patch: applied.
mc-3641-cleanup-Wformat-signedness-hex_c.patch: applied with extra small change.
mc-3641-cleanup-Wformat-signedness-info_c.patch: applied.
mc-3641-cleanup-Wformat-signedness-midnight_c.patch: applied.
mc-3641-cleanup-Wformat-signedness-path_c.patch: another way was used.
mc-3641-cleanup-Wformat-signedness-serialize_c.patch: applied.
mc-3641-cleanup-Wunsafe-loop-optimizations.patch: applied.
mc-3641-cleanup-no-attribute-noreturn-utilunix.c.patch: applied.
mc-3641-cleanup-no-attribute-noreturn-utilvfs.c.patch: applied.
mc-3641-cleanup-unreachable-code-warning-utilunix_c.patch: applied.

comment:2 Changed 5 months ago by egmont

  • Cc egmont added

comment:3 Changed 4 months ago by andrew_b

  • Blocking 3703 added

Changed 4 months ago by mooffie

comment:4 Changed 4 months ago by andrew_b

3693-Hex-patterns-fix-manual-page.patch​: applied.

comment:5 Changed 4 months ago by mooffie

FWIW: I've read all the commits in this branch (except "Clarify startup" and "fix location of right panel", for lack of time) and they seem alright.

(As for ditching negative numbers in hex patterns (hex.c): that's a blessing (at least as long as nobody volunteers to implement reading words/dwords/etc). Besides, reading them with "%x" meant that if you typed "-10" you got -16.)

Last edited 4 months ago by mooffie (previous) (diff)

comment:6 follow-up: ↓ 7 Changed 4 months ago by mooffie

@Andrew:

re: "Clarify usage of assert":

There are two places that use g_assert() (and 3 places that use g_assert_*()).

The purpose of AC_HEADER_ASSERT is to make --disable-assert define the NDEBUG constant, which disables assert(). But to disable g_assert() one has to define the G_DISABLE_ASSERT constant.

So, it seems to me, we should choose either assert() or g_assert(), not both.

It's easier to just rename the two g_assert() we have to assert().

(One may argue that there are a few g_assert_*() functions we may covet (like g_assert_cmpint), and that for the sake of stylistic uniformity we should therefore prefer g_assert() to assert(). But these g_assert_*() bunch can't be disabled, so we may treat them as a category of their own.)

Not that I care too much about any of this. It's just that I'm about to propose a patch in which I was planning to use g_assert() / assert() and I don't want to get some flak ;-)

comment:7 in reply to: ↑ 6 Changed 4 months ago by andrew_b

Replying to mooffie:

So, it seems to me, we should choose either assert() or g_assert(), not both.

Indeed.

I prefer g_assert() and I've added the commit.

comment:8 Changed 4 months ago by zaytsev

@andrew_b: just a nitpick, the plural of assert is not "Asserts:", but "Assertions:".

comment:9 Changed 4 months ago by zaytsev

Here too: "Define to disable asserts" -> "Define to disable assertions".

comment:10 Changed 4 months ago by andrew_b

Rebased to current master: [e036c1fb12757dc9d258862736054c33313a51d5].

comment:11 Changed 4 months ago by andrew_b

  • Branch state changed from no branch to on review

Changed 3 months ago by mooffie

This documentation is needed in order to understand the bug(s) in #3720.

comment:12 Changed 3 months ago by andrew_b

3693-mc_search_run-document-the-return-value.patch​: applied.

Rebased to current master: [645e88d209a520e6c83eeced710b9376f878c8f8].

comment:13 Changed 3 months ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:14 Changed 3 months ago by andrew_b

  • Votes for changeset changed from andrew_b to committed-master
  • Branch state changed from approved to merged

Merged to master: [24a16003a8c01faaeb4e907e674c7bddb36e1424].

git log --pretty=oneline da538f0..24a1600

comment:15 Changed 3 months ago by andrew_b

  • Blocking 3703 removed

comment:16 Changed 3 months ago by andrew_b

  • Votes for changeset committed-master deleted
  • Branch state changed from merged to no branch

Next series: 3693_cleanup
Initial changeset:9231e10791fa313a04afcf7eb78e671ea0ac3d47

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

Changed 3 months ago by mooffie

(This documentation is needed to better understand the patch I'll have for #3589.)

comment:17 Changed 3 months ago by andrew_b

3693-mc_search__cond_struct_new_regex_ci_str-add-documentation.patch: applied. Thanks!

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

comment:18 Changed 3 months ago by mooffie

The following patch is too trivial for a separate ticket. It improves the highighting of comments in some syntax files.

# list all syntax files with comments not on 1st column:

$ grep -P '^\s+#' -l *

as.syntax
ebuild.syntax
jal.syntax
js.syntax
lkr.syntax
lua.syntax
m4.syntax
yaml.syntax

(The patch doesn't add support for comments preceded by TABs, though. But that's no biggie.)

comment:19 Changed 3 months ago by andrew_b

3693-mcedit-syntax.syntax-highlight-comments-preceded-by-spaces.patch: applied.

Changed 3 months ago by and

comment:20 follow-up: ↓ 26 Changed 3 months ago by andrew_b

mc-3693-dialogs.c-Cleanup-maybe-uninitialized-compiler-warni.patch​: [1]
mc-3693-edit.c-Cleanup-unsafe-loop-optimizations-compiler-wa.patch​: [2]
mc-3693-editcmd.c-Cleanup-comma-compiler-warning.patch​: [3]
mc-3693-editcmd_dialogs.c-Cleanup-maybe-uninitialized-compil.patch​: [1]
mc-3693-execute.c-Cleanup-maybe-uninitialized-compiler-warni.patch​: [1]
mc-3693-file.c-Cleanup-maybe-uninitialized-compiler-warning.patch​: [1]
mc-3693-internal.c-Cleanup-format-warning.patch​: applied.
mc-3693-lib.c-Cleanup-comma-compiler-warning.patch​: [3]
mc-3693-main.c-Cleanup-maybe-uninitialized-compiler-warning.patch: [1]
mc-3693-panel.c-Cleanup-comma-compiler-warning.patch​: [3]
mc-3693-search.c-Cleanup-maybe-uninitialized-compiler-warnin.patch​: [1]
mc-3693-syntax.c-Cleanup-unsafe-loop-optimizations-compiler-.patch: [2]

[1] squashed in the single commit.
[2] I can't see something wrong in these loops.
[3] squashed in the single commit.

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

comment:21 Changed 3 months ago by andrew_b

We have strange build error:

  CC     file.lo
../../../../src/vfs/sftpfs/file.c: In function ‘sftpfs_open_file’:
../../../../src/vfs/sftpfs/file.c:156:16: error: missing initializer [-Werror=missing-field-initializers]
../../../../src/vfs/sftpfs/file.c:156:16: error: (near initialization for ‘file_info.st_ino’) [-Werror=missing-field-initializers]

gcc-4.6.3 is used.

The code is following:

155     {
156         struct stat file_info = { 0 };
157 
158         if (sftpfs_fstat (file_handler, &file_info, mcerror) == 0)
159             libssh2_sftp_seek64 (file_handler_data->handle, file_info.st_size);
160     }

What's wrong here?

My local build with gcc-5.3.1 is finished successfully.

comment:22 follow-up: ↓ 23 Changed 3 months ago by zaytsev-work

Try struct stat file_info = { }; ? I think that it complains, because if you specify the first 0, then it thinks that you should you should go all the way and specify all of them, but this annoying behaviour for this warning was changed in later GCCs.

comment:23 in reply to: ↑ 22 Changed 3 months ago by andrew_b

Replying to zaytsev-work:

Try struct stat file_info = { }; ?

This was in the original patch. The result was same:

  CC     file.lo
../../../../src/vfs/sftpfs/file.c: In function ‘sftpfs_open_file’:
../../../../src/vfs/sftpfs/file.c:156:16: error: missing initializer [-Werror=missing-field-initializers]
../../../../src/vfs/sftpfs/file.c:156:16: error: (near initialization for ‘file_info.st_dev’) [-Werror=missing-field-initializers]

comment:24 follow-up: ↓ 25 Changed 3 months ago by zaytsev-work

Oh, sorry, stupid me, struct stat file_info = { }; isn't even valid C code according to the standard (which can be witnessed via -pedantic):

http://stackoverflow.com/questions/17589533/is-an-empty-initializer-list-valid-c-code

It's only valid in C++, so your change to struct stat file_info = { 0 }; is absolutely correct.

Anyways, I have found the GCC bug report regarding { 0 } in C, and, unfortunately, it only made it in GCC 4.7.x:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36750

However, it seems a workaround for GCC 4.6.x is possible:

http://stackoverflow.com/a/27461062

struct stat file_info = { .st_dev = 0 };

Maybe this will work? The only other thing that comes to mind is to mess with pragmas, but it smells so badly that I wouldn't consider this as an option... :-(

comment:25 in reply to: ↑ 24 Changed 3 months ago by andrew_b

Replying to zaytsev-work:

Maybe this will work?

Yes, it works. Thanks!

comment:26 in reply to: ↑ 20 ; follow-up: ↓ 27 Changed 3 months ago by and

Replying to andrew_b:

mc-3693-edit.c-Cleanup-unsafe-loop-optimizations-compiler-wa.patch​: [2]

[2] I can't see something wrong in these loops.

GCC complains about unsafe loop optimization when "... <= VALUE" found,
because internal comparing works like "... < (VALUE+1)" and possible VALUE=MAX_INT
will overflow, hence unsafe loop optimization possible.

Last section at
https://stackoverflow.com/questions/2982507/what-is-the-explanation-for-warning-assuming-that-the-loop-is-not-infinite

comment:27 in reply to: ↑ 26 Changed 2 months ago by andrew_b

Replying to and:

Last section at
https://stackoverflow.com/questions/2982507/what-is-the-explanation-for-warning-assuming-that-the-loop-is-not-infinite

I found this webpage too. I think GCC is some paranoiac here.

comment:28 Changed 2 months ago by andrew_b

  • Branch state changed from no branch to on review

comment:29 Changed 4 weeks ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:30 Changed 4 weeks ago by andrew_b

  • Votes for changeset changed from andrew_b to committed-master
  • Branch state changed from approved to merged

Merged to master: [9845fa6ce542c5c45ab9a86140052dd3ded03655].

git log --pretty=oneline 43d7fce..9845fa6

comment:31 Changed 3 weeks ago by mooffie

I'm attaching the patch that was left out of #3571. I published it in #3760 a few days ago but it turns out it's no longer needed there. So I put it here. If you don't want to commit it then that's fine with me (although I suspect this will bite us sooner or later).

Changed 3 weeks ago by mooffie

Note: See TracTickets for help on using tickets.