Ticket #3693 (new task)

Opened 4 months ago

Last modified 2 days 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 4 months ago.
mc-3641-cleanup-Wformat-signedness-editdraw_c.patch (3.4 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-file_c.patch (1.2 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-find_c.patch (2.6 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-ftpfs_c.patch (1.1 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-hex_c.patch (1.2 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-info_c.patch (3.1 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-midnight_c.patch (1.7 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-path_c.patch (1.0 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wformat-signedness-serialize_c.patch (4.5 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-Wunsafe-loop-optimizations.patch (1.1 KB) - added by andrew_b 4 months ago.
mc-3641-cleanup-no-attribute-noreturn-utilunix.c.patch (765 bytes) - added by andrew_b 4 months ago.
mc-3641-cleanup-no-attribute-noreturn-utilvfs.c.patch (1005 bytes) - added by andrew_b 4 months ago.
mc-3641-cleanup-unreachable-code-warning-utilunix_c.patch (930 bytes) - added by andrew_b 4 months ago.
3693-Hex-patterns-fix-manual-page.patch (1.2 KB) - added by mooffie 3 months ago.
3693-mc_search_run-document-the-return-value.patch (1.1 KB) - added by mooffie 2 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 8 weeks 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 7 weeks ago.
mc-3693-dialogs.c-Cleanup-maybe-uninitialized-compiler-warni.patch (973 bytes) - added by and 7 weeks ago.
mc-3693-edit.c-Cleanup-unsafe-loop-optimizations-compiler-wa.patch (1.3 KB) - added by and 7 weeks ago.
mc-3693-editcmd.c-Cleanup-comma-compiler-warning.patch (1.5 KB) - added by and 7 weeks ago.
mc-3693-editcmd_dialogs.c-Cleanup-maybe-uninitialized-compil.patch (1.6 KB) - added by and 7 weeks ago.
mc-3693-execute.c-Cleanup-maybe-uninitialized-compiler-warni.patch (1.1 KB) - added by and 7 weeks ago.
mc-3693-file.c-Cleanup-maybe-uninitialized-compiler-warning.patch (1.0 KB) - added by and 7 weeks ago.
mc-3693-internal.c-Cleanup-format-warning.patch (981 bytes) - added by and 7 weeks ago.
mc-3693-lib.c-Cleanup-comma-compiler-warning.patch (1.2 KB) - added by and 7 weeks ago.
mc-3693-main.c-Cleanup-maybe-uninitialized-compiler-warning.patch (982 bytes) - added by and 7 weeks ago.
mc-3693-panel.c-Cleanup-comma-compiler-warning.patch (877 bytes) - added by and 7 weeks ago.
mc-3693-search.c-Cleanup-maybe-uninitialized-compiler-warnin.patch (943 bytes) - added by and 7 weeks ago.
mc-3693-syntax.c-Cleanup-unsafe-loop-optimizations-compiler-.patch (1.5 KB) - added by and 7 weeks ago.

Change History

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

Changed 4 months ago by andrew_b

comment:1 Changed 4 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 4 months ago by egmont

  • Cc egmont added

comment:3 Changed 3 months ago by andrew_b

  • Blocking 3703 added

Changed 3 months ago by mooffie

comment:4 Changed 3 months ago by andrew_b

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

comment:5 Changed 3 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 3 months ago by mooffie (previous) (diff)

comment:6 follow-up: ↓ 7 Changed 3 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 3 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 3 months ago by zaytsev

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

comment:9 Changed 3 months ago by zaytsev

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

comment:10 Changed 3 months ago by andrew_b

Rebased to current master: [e036c1fb12757dc9d258862736054c33313a51d5].

comment:11 Changed 3 months ago by andrew_b

  • Branch state changed from no branch to on review

Changed 2 months ago by mooffie

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

comment:12 Changed 2 months ago by andrew_b

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

Rebased to current master: [645e88d209a520e6c83eeced710b9376f878c8f8].

comment:13 Changed 2 months ago by andrew_b

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

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

  • Blocking 3703 removed

comment:16 Changed 2 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 6 weeks ago by andrew_b (previous) (diff)

Changed 8 weeks ago by mooffie

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

comment:17 Changed 8 weeks ago by andrew_b

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

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

comment:18 Changed 7 weeks 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 7 weeks ago by andrew_b

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

Changed 7 weeks ago by and

comment:20 follow-up: ↓ 26 Changed 7 weeks 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 6 weeks ago by andrew_b (previous) (diff)

comment:21 Changed 7 weeks 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 7 weeks 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 7 weeks 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 7 weeks 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 7 weeks 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 7 weeks 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 6 weeks 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 4 weeks ago by andrew_b

  • Branch state changed from no branch to on review

comment:29 Changed 2 days ago by andrew_b

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

comment:30 Changed 2 days 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
Note: See TracTickets for help on using tickets.