[PATCH] Micro helper library

[Enrico.Weigelt@…>]

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

Committed to master:
changeset:4765514421d29983040a1d6a51789c7d4cdf4342

Committed to mc-4.6:
changeset:1e2ed2f808c79b101d2ca70d3f25a3db41eb77b9

[anonymous]

Added by email2trac

[anonymous]

This message has 1 attachment(s)

[andrew_b]

Replying to Enrico.Weigelt@…>:

this patch introduces some new mhl/*.h

IMHO, 'mhl' name is not intuitively :)

[Enrico Weigelt]

• MC Ticket System <tickets@…> schrieb:

  IMHO, 'mhl' name is not intuitively :)
  

you have a better idea ? ;-p

[andrew_b]

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

[slavazanko]

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

[slavazanko]

in addition to my previous comment:

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

[winnie]

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.

[slavazanko]

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

[slavazanko]

extend mhl: use glib (if #defined WITH_GLIB)

[slavazanko]

Changes from rev001 to rev002 between patches for better review

[slavazanko]

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.

[metux]

@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.

[winnie]

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.

[slavazanko]

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. :)

[slavazanko]

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

[slavazanko]

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; changeset:51a81ec6e3191a693d09a20e337056055bf1daef

Or may exists other way?

[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! :)

[ossi]

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.

[Enrico Weigelt]

• MC Ticket System <tickets@…> 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

[winnie]

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. :)

[winnie]

Closing. Remote branch is removed.