[PATCH] Micro helper library

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/157
Reporter	Enrico.Weigelt@….local, metux IT service <weigelt@….de>
Keywords	vote-winnie, vote-slavazanko, committed-master, committed-mc-4.6

this patch introduces some new mhl/*.h files with things like (safer) memory management and string handling, shell escaping, etc

Committed to master:
[4765514]

Committed to mc-4.6:
[1e2ed2f]

Note

Original attachments:

• mhl.001.diff (raw) by on Jan 9, 2009 at 19:33 UTC
• mhl.002.diff (raw) by slavazanko (@slavaz) on Jan 13, 2009 at 9:55 UTC
• mhl.from001-to002.diff (raw) by slavazanko (@slavaz) on Jan 13, 2009 at 9:56 UTC
• own_inline.patch (raw) by slavazanko (@slavaz) on Jan 14, 2009 at 8:38 UTC

[mc-butler]

Changed by mc-butler (@mc-butler) on Jan 9, 2009 at 19:33 UTC

• Attachment mhl.001.diff (raw) added
• Id set to 157

This message has 1 attachment(s)

[mc-butler]

Changed by metux (@metux) on Jan 9, 2009 at 20:22 UTC (comment 2)

• Keywords set to review
• Type changed from defect to enhancement
• Description edited
• Blocking set to #10, #14, #41, #55, #81, #125, #147, #149, #152

[mc-butler]

Changed by andrew_b (@aborodin) on Jan 10, 2009 at 16:13 UTC

Replying to Enrico.Weigelt@…> (#157):

this patch introduces some new mhl/*.h

IMHO, 'mhl' name is not intuitively :)

[mc-butler]

Changed by metux (@metux) on Jan 10, 2009 at 16:34 UTC (comment 4)

• Status changed from new to accepted
• Owner set to metux

[mc-butler]

Changed by Enrico Weigelt on Jan 11, 2009 at 17:11 UTC

• MC Ticket System <​tickets@midnight-commander.org> schrieb:

  IMHO, 'mhl' name is not intuitively :)
  

you have a better idea ? ;-p

[mc-butler]

Changed by andrew_b (@aborodin) on Jan 13, 2009 at 3:20 UTC (comment 6)

• Keywords changed from review to rework

Type corrections are needed: strlen() returns a value of type size_t, not int.

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 13, 2009 at 8:13 UTC (comment 7)

What about $(src_root)/lib directory?

Why is it necessary to create a directory 'mhl', if there is a directory 'lib'?

My propose:

git-mv lib contrib
git-mv mhl lib
git-commit

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 13, 2009 at 8:15 UTC (comment 8)

in addition to my previous comment:

... and, of course, change files configure.ac and Makefile.am for correctly compile with new changes.

[mc-butler]

Changed by winnie (@winnieXY) on Jan 13, 2009 at 8:23 UTC (comment 9)

Well.. as this should go to 4.6.2 which is only a bugfixing release I doesn't want to restructure the code here.

We can restructurize the code afterwards in a new ticket ( I think there is already a ticket about restructurizing the code)... This would then belong into this ticket not into this one.

I would now (for the 4.6.X branch) leave mhl where it is (or move it to lib/mhl but leaving the rest as it is.

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 13, 2009 at 9:54 UTC (comment 10)

Ah... if after release-4.6 out mhl will move to lib - I agree :)

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 13, 2009 at 9:55 UTC

• Attachment mhl.002.diff (raw) added

extend mhl: use glib (if #defined WITH_GLIB)

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 13, 2009 at 9:56 UTC

• Attachment mhl.from001-to002.diff (raw) added

Changes from rev001 to rev002 between patches for better review

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 13, 2009 at 11:13 UTC (comment 11)

oops...

mhl/memory.h:4

#if defined _AIX && !defined REGEX_MALLOC 
#pragma alloca 
#endif 

REGEX_MALLOC must be removed. This stuff copyed from src/regex.c (for correctly declate 'alloca' function).

In any case, patch need to review and rework.

[mc-butler]

Changed by metux (@metux) on Jan 13, 2009 at 17:43 UTC (comment 12)

@Slava: your alloca() changes are very fine, but:

a) the glib stuff is quite useless, just adds extra code without any benefit and makes the binary more expensive (adds farcalls to funcs which far-call funcs that could have been inlined)

b) moving the funcs to separate .c file just kills inline'ing.

[mc-butler]

Changed by winnie (@winnieXY) on Jan 13, 2009 at 21:58 UTC (comment 13)

• Milestone changed from 4.7 to 4.6.2

Setting milestone to 4.6.2 as we noone on the ML said anything against this and we really need parts of this patch in order to fix other important stuff.

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 14, 2009 at 8:36 UTC (comment 14)

a) the glib stuff is quite useless, just adds extra code

On a modern computers it's don't important :) For other systems we make own realization of glib-functions... if needed :)

b) moving the funcs to separate .c file just kills inline'ing.

Modern compilers may automatically make some function inline or not. In mc-ru-fork we was attempt to use own standart _GNU_INLINE_ (make some checks in configure-script)

See next attach.

Some example of usage:

_GNU_INLINE_ void _ATTRIBUTE_ALWAYS_INLINE_
some_function_1(params){
   ...
}
static _GNU_INLINE_ char *
some_function_2(params){
   ...
}
...

For more info about this need to consult with Pavlinux(Russian team of mc-ru-fork)... but he not present in this trac... :(

P.S. I will return inline'ing to functions back. :)

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 14, 2009 at 8:38 UTC

• Attachment own_inline.patch (raw) added

Some checks for GNU-extensions in compiler for support inline'ing

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 14, 2009 at 8:39 UTC (comment 15)

• Priority changed from major to critical

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 14, 2009 at 10:54 UTC (comment 16)

BTW, in mhl/string.c some warnings:

string.c: In function ‘mhl_str_reverse’:
string.c:38: warning: implicit declaration of function ‘strlen’
string.c:38: warning: incompatible implicit declaration of built-in function ‘strlen’
...

adding string to a top of mhl/string.c:

#include <string.h>

don't avoid warnings. Because one of current includedir in CFLAGS is a: -I./
I think, need to rename mhl/string.[ch] to mhl/mhl_string.[ch]
This avoid warnings. See branch; [51a81ec]

Or may exists other way?

[mc-butler]

Changed by winnie (@winnieXY) on Jan 15, 2009 at 13:49 UTC (comment 17)

• Keywords changed from rework to review, vote-winnie

Hey,

After a discussion with slavaz on jabber I created a new branch called 157_mhl_additions_to_master which the stuff recently added by him. After that I've reverted the stuff from slavaz in the old branch so that we can get a minimalistic stuff into 4.6.2 to fix there some problems.

So please have _now_ a look on the 157_mhl_addition branch and vote! :)

[mc-butler]

Changed by ossi (@ossilator) on Jan 15, 2009 at 21:26 UTC (comment 14.18)

Replying to slavazanko:

b) moving the funcs to separate .c file just kills inline'ing.

Modern compilers may automatically make some function inline or not.

uhm, so what? moving to a separate .c file still kills inlining. period.

[mc-butler]

Changed by Enrico Weigelt on Jan 16, 2009 at 8:21 UTC

• MC Ticket System <​tickets@midnight-commander.org> schrieb:

Comment(by slavazanko):

a) the glib stuff is quite useless, just adds extra code

On a modern computers it's don't important :)

Useless code is always a mess, at least for maintenance.
I'm really curios which problem you intend to solve - or is it
just glib-fetishism ? ;-o

For other systems we make own realization of glib-functions... if needed :)

We dont need any extra solution, just leave it as it was.

b) moving the funcs to separate .c file just kills inline'ing.

Modern compilers may automatically make some function inline or not.

Only if they can. As soon as you put the code in an extra .c file, which
gets compiled separately, the compiler has no chance to do inline'ing.

Some example of usage:

_GNU_INLINE_ void _ATTRIBUTE_ALWAYS_INLINE_
some_function_1(params){
   ...
}

This would only make sense if you want to *enforce* inline'ing on
an per-function basis. Is this really necessary ? Do you really feel
more clever than the compiler on whether to inline certain specific
function ?

cu

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 17, 2009 at 19:15 UTC (comment 20)

• Keywords changed from review, vote-winnie to review, vote-winnie, vote-slavazanko

[mc-butler]

Changed by winnie (@winnieXY) on Jan 18, 2009 at 9:58 UTC (comment 21)

• Keywords changed from review, vote-winnie, vote-slavazanko to review, vote-winnie, vote-slavazanko, approved

Cool.. this is approved now.

@metux: Could you please merge this into mc-4.6 and master? After that I'll work on the whitespace issues and completion. :)

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 18, 2009 at 11:00 UTC (comment 22)

• Keywords changed from review, vote-winnie, vote-slavazanko, approved to vote-winnie, vote-slavazanko, approved

[mc-butler]

Changed by slavazanko (@slavaz) on Jan 18, 2009 at 11:05 UTC

• Blocking changed from #10, #14, #41, #55, #81, #125, #147, #149, #152 to #10, #14, #41, #55, #81, #125, #147, #149, #152, #179

[mc-butler]

Changed by metux (@metux) on Jan 19, 2009 at 1:35 UTC (comment 24)

• Keywords changed from vote-winnie, vote-slavazanko, approved to vote-winnie, vote-slavazanko, committed-master, committed-mc-4.6
• Status changed from accepted to testing
• Resolution set to fixed
• Description edited

[mc-butler]

Changed by winnie (@winnieXY) on Jan 19, 2009 at 6:45 UTC (comment 25)

• Status changed from testing to closed

Closing. Remote branch is removed.

[mc-butler]

Changed by andrew_b (@aborodin) on May 10, 2010 at 9:51 UTC

• Blocking changed from #10, #14, #41, #55, #81, #125, #147, #149, #152, #179 to #10, #14, #41, #55, #125, #147, #149, #152, #179

[mc-butler]

Changed by andrew_b (@aborodin) on Jun 21, 2010 at 7:32 UTC

• Blocking changed from #10, #14, #41, #55, #125, #147, #149, #152, #179 to #10, #14, #41, #125, #147, #149, #152, #179

[mc-butler]

Changed by andrew_b (@aborodin) on Jun 21, 2010 at 7:33 UTC

• Blocking changed from #10, #14, #41, #125, #147, #149, #152, #179 to #10, #14, #125, #147, #149, #152, #179

[mc-butler]

Changed by andrew_b (@aborodin) on Jun 28, 2010 at 6:49 UTC

• Blocking changed from #10, #14, #125, #147, #149, #152, #179 to #10, #14, #125, #147, #152, #179

[mc-butler]

Changed by zaytsev (@zyv) on Feb 22, 2025 at 12:18 UTC

• Blocking changed from #10, #14, #125, #147, #152, #179 to #10, #14, #125, #147, #149, #152, #179