hnes / libaco

A blazing fast and lightweight C asymmetric coroutine library 💎 ⛅🚀⛅🌞
https://libaco.org
Apache License 2.0
3.49k stars 392 forks source link

Add aco_mem_new macro #23

Open platipo opened 5 years ago

platipo commented 5 years ago

I followed your coding convention, but I think that lowercase macros are evil because it confuses newcomers, uppercase macros are convention.

I took the freedom to add a new macro aco_mem_calloc which you can read more details in the commit.

Also I want to point out that this commit "breaks" C++ support, which was already broken by _Static_assert, because using malloc in C++ is a really bad habit and there is no support for typeof of something similar in C++.

Issue #3.

platipo commented 5 years ago

One workaround if you wanna stick with malloc could be:

# define aco_mem_new(ptr, size) do { \
    ptr = static_cast<decltype(ptr)>(malloc(size)); \
    assertalloc_ptr(ptr); \
} while (0);
# define aco_mem_calloc(ptr, count) do { \
    ptr = static_cast<decltype(ptr)>(calloc(count, sizeof(*ptr))); \
    assertalloc_ptr(ptr); \
} while (0);

instead of #error

hnes commented 5 years ago

Thank you very much for this PR, @platipo :D

I am very sorry to reply so late.

I followed your coding convention, but I think that lowercase macros are evil because it confuses newcomers, uppercase macros are convention.

Yes, but I'm afraid that I only agree with you partly. In the Code Style of linux kernel there is:

CAPITALIZED macro names are appreciated but macros resembling functions may be named in lower case.

Which is also the convention been used in the libaco. In this case, the aco_mem_new(ptr, sz) in this PR may should be changed to the form of ptr = (type_cast) aco_mem_new(sz) and aco_mem_calloc should be similar. We may should also implement them as "static inline functions" because the statement expressions is not a part of C standard.

I took the freedom to add a new macro aco_mem_calloc which you can read more details in the commit.

I very appreciate your idea. For many situations, "calloc" is indeed usually more efficient than "malloc + memset".

Also I want to point out that this commit "breaks" C++ support, which was already broken by _Static_assert, because using malloc in C++ is a really bad habit and there is no support for typeof of something similar in C++.

I don't think the _Static_assert would break the C++ support because:

And finally, please forgive me that I would like to deal with this PR after the commit of issue #22 since it has the highest priority ;-)

platipo commented 5 years ago

We may should also implement them as "static inline functions" because the statement expressions is not a part of C standard.

I think that's the best thing to do.

Seeing lowercase macros annoys me because I have always seen capitalized macro names even in function like macros and whats really bothers me is macro expansion. It's notorious that macros may affect things you don't realize and can lead to strange side effects, indeed we are using them as functions but they don't have return like behavior. So I think its needed to emphasize their usage by al least uppercasing their names.

Our user should compile libaco with a C compiler since libaco is a C library.

If so why casting void pointers, it's not correct in C because they are automatically and safely promoted to any other pointer type.

And finally, please forgive me that I would like to deal with this PR after the commit of issue #22 since it has the highest priority ;-)

Of course :smiley: