Summary:
This was causing problems on non-Linux architectures as eina_file_real_close unmapped not mapped data. Added a "copied" flag to Eina_File which is set on eina_file_virtualize (on copied data), and tested for when eina_file_real_close does the unmap. I'm surprised Linux allowed this. Certainly all of the BSDs crashed with the previous behaviour.
@fix T5479
Test Plan: Example inlcude Rage and Enlightenment Thumb on BSD systems which use eina_file_virtualize with emotion to obtain album artwork.
Reviewers: raster, cedric, jpeg
Reviewed By: jpeg
Maniphest Tasks: T5479
Differential Revision: https://phab.enlightenment.org/D5006
so we copy data to an UNALINED memory address (just after whatever
string we packed on the end of the eina file struct header). this is
bad. especially for non-intel architectures. this forces a 16 byte
alignment which should cover us.
@fix
I'm not sure about the rest of this code, so it's possible that
the index is increased even if it shouldn't. But I've observed
a crash at this line, apparently when reaching the end pointer.
There was trouble with Homebrew's CI to build EFL on a macOS < 10.12
which uses a 10.12 SDK. See PR #13252 on github, Homebrew/homebrew-core
for details.
@fix
The opcodes registration request is sent directly in case the connection
is already made. Otherwise, the request is waiting for the connection to
be made by the dedicated thread (not the main loop).
That's why the request can be sent by the two different threads at the
same time, leading to send it twice. It means a callback for an opcode
would be invoked twice everytime a request with this opcode is received.
This patch fixes it by checking if the request has already been sent.
GCC has started introducing a detection for series of case in a switch statement
without break for each case. We do use that trick a lot to reduce our code base.
Even if in most case we have documented this so that people using coverity don't
try to fix it. Now with GCC we need to silence it properly to avoid future
problem.
Eina Debug is a new layer aimed for EFL debugging. It offers scalability
by allowing registration of operations not specific to EFL core.
The protocol is simple and the APIs try to provide as much
functionalities/freedom as possible.
Summary:
Escaping is not happening whenever any escapable characters is coming after
'\t' or '\n'. It will also fix invalid read of 1 byte which happens for string where
last charachter is '\t' or '\n' like "eina\t".
Test Plan:
Take a string like "eina\t ". Observe space which is followed by tab is not getting
escaped.
Signed-off-by: Prasoon Singh <prasoon.16@samsung.com>
Reviewers: shilpasingh, rajeshps, govi, cedric
Reviewed By: shilpasingh
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4847
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
since this code's creation it seems that the internal int size was set to use
short in order to micro-optimize memory usage, while the api function parameters
used Eina_Rectangle which had a larger int size. when initializing the internal
rect struct, this would lead to overflows which resulted in broken tilers which
returned iterators with no valid rects after having valid rects added
test case: run weston-subsurfaces
@fix
it seems coverity didn't like our checks like if end - start > 0xffff
then dont do anything. this should effectively stop any issues but
seemingly not, so try another way to keep coverity happy.
CID 1361219
it seems coverity didn't like our checks like if end - start > 0xffff
then dont do anything. this should effectively stop any issues but
seemingly not, so try another way to keep coverity happy.
CID 1361220
Summary:
There should be reallocation +1 (for last '\0') and also
checking >0, not !=0, because of getxattr can return -1 in case of error
@fix
Reviewers: cedric, raster, NikaWhite, jpeg
Reviewed By: NikaWhite
Subscribers: myoungwoon
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D4734
The eina_(rw)_slice_startswith functions both incorrectly describe how
the 'prefix' parameter is used, so fix those also
@fix
Signed-off-by: Chris Michael <cp.michael@samsung.com>
eina_(rw)_slice_startswith functions both incorrectly describe the
return value as 'slice ends with', when clearly the function is used
to find if a slice 'starts with' a prefix
@fix
Signed-off-by: Chris Michael <cp.michael@samsung.com>
Summary: CreateFileMapping return handle. The handle before use is always closed. This handle can be immediately closed after use.
Reviewers: cedric, raster, vtorri, rimmed, an.kroitor, FurryMyad, NikaWhite
Reviewed By: raster
Subscribers: artem.popov, cedric, jpeg
Tags: #windows
Differential Revision: https://phab.enlightenment.org/D4699
Summary:
The usage of the macro EINA_MAGIC_CHECK_LIST can
lead (in some cases) to leaks.
Signed-off-by: Flavio Ceolin <flavio.ceolin@gmail.com>
Reviewers: jpeg
Reviewed By: jpeg
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4679
Summary: I had fixed some typos and some wrong expressions, such as capital letters, singular, and orders of groups in Eina API reference doxygen.
Test Plan: Doxygen Revision
Reviewers: stefan, cedric, raster, Jaehyun_Cho, jpeg
Reviewed By: jpeg
Subscribers: conr2d
Differential Revision: https://phab.enlightenment.org/D4674
If HAVE_GETPWENT isn't defined - the closing brace was missed.
Also prevent situation when strdup() tried to duplicate NULL
pointer, because that could cause segfault.
@fix
It was only defined in the c file. Without any documentation, since tag, etc.
tests/eina/eina_test_file.c:855:4: warning: implicit declaration of function ‘eina_file_unlink’
[-Wimplicit-function-declaration]
we really can't do much here but our direct casting causes warnings in
apps or anyone using this macro so keep things silent as our pointer
tricks are actually ok but the compiler can't figure it out.
if setuod we dont want to trust HOME environ at all and get it from
passwd file... also we dont want to keep re-getting too... so store
statically as well as tmp.
this also kind of helps CID 1366469
we set stack var to 0 even if evlog was off and thus didn't use it.
this cleans up the evlog func a bit and also moves locking until later
so it's locked for the minimum period to punt something into the log
buffer. it's an improvement, but no bug fix.
If we're not logging events this generates a lot of wasted system
calls. They probably don't amount to much, but it's trivial to
get rid of them, and they make a mess when logging with strace.
This is now used by ENABLE_SYSTEMD and ENABLE_VALGRIND, which moves to
"common.cmake" since they are shared among multiple libraries.
With that I found that LINK_FLAGS is indeed a string, not a CMake List
(space separated, not ";"), then fix that so compilation actually works.
make FUNC_CHECK(), TYPE_CHECK() and HEADER_CHECK() more general and
they can be set to a scope, like "eina", then all symbols are prefixed
with that. The scope is created with CHECK_INIT(), and
EFL_HEADER_CHECKS_FINALIZE() will finish that.
This makes it possible for cmake/config/eina.cmake +
cmake/post/eina.cmake to add stuff to the generated file, better than
hand edit the template.
CHECK_APPEND_DEFINE(name val) is now the base to add symbols to the
generated file in the current scope.
Then convert cmake/config/eina.cmake to use that and match the
autotools values (were a bit off).
This exposed enabling valgrind was broken due incorrect pkg-config
usage with cmake (it's not obvious), it was using just the libraries,
while LDFLAGS are needed to get -L/usr/lib/valgrind. Then also convert
to CFLAGS provided by pkg-config and make that automatic for
PKG_CONFIG_REQUIRES and PKG_CONFIG_REQUIRES_PRIVATE.
Also, eina-mempool modules use valgrind and must use that now that's
propagating correctly.
For one-source directories, be smart and just define SOURCES to that,
will reduce the number of too-simplistic CMakeLists.txt in our tree.
This also fixes problems with libraries, they should be private, not
public. So specify both kinds as different variables.
Stick to one target per directory and remove prefix from variables,
makes it cleaner and easier to use.
Document variables used and use a more consistent name that matches
CMake properties.
I believe this function is not required and should not be
used by applications. If there is a very good use case to
use your own main freeq, then the API could be added again.
For now, removing the set() is probably the safer option.
Note: the API was introduced in the upcoming 1.19
Built on top of the new 'postponed' free queue, the short-lived
strings API allows users to return new strings without caring
about freeing them. EFL main loop will do this automatically for
them you at a later point in time (at the end of an iteration).
The APIs provided will either duplicate (copy) or more generally
steal an existing string (char *, stringshare, tmpstr, strbuf),
taking ownership of it and controling its lifetime. Those strings
can then be safely returned by an API. From a user point of view,
those strings must be considered like simple const char *, ie.
no need to free() them and their validity is limited to the
local scope.
There is no function to remove such a string from the freeq.
The short lived strings API is not thread-safe: do not send a
short-lived object from one thread to another.
@feature
While this reuses the existing (but new) infrastructure of
eina_freeq, the mode of operation and objective is very different
from the default freeq.
By default, any object added to the freeq is basically already
freed from the user point of view, and the freeq itself only adds
a tiny layer of memory safety by deferring the actual call to free
and optionally filling the memory blob with a pattern ('wwwww...').
This is mostly thread-safe (requires thread-safe free functions).
This new type I called postponed is intended to store objects that
will be short lived. This is not thread safe as the life of the
objects added to this queue depends on the thread that adds to
the queue. The main intent is to introduce a new API for short-lived
strings.
@feature
The api name free_return wasnt a good choice so it is changed to
release. This also moves the implementation to binbuf template so it is
available in all buf types.
Summary:
For a function which just composes a string with strbuf its quite
usefull to return the string while its freed.
This makes a function like:
{
Eina_Strbuf *buf;
char *path;
buf = eina_strbuf_new();
eina_strbuf_append(buf, "test");
eina_strbuf_append_printf(buf, "%s-%d.edj", "test", 0);
path = eina_strbuf_string_steal(buf);
eina_strbuf_free(buf);
return path;
}
To:
{
Eina_Strbuf *buf;
buf = eina_strbuf_new();
eina_strbuf_append(buf, "test");
eina_strbuf_append_printf(buf, "%s-%d.edj", "test", 0);
return eina_strbuf_free_return(buf);
}
Which is a bit more handy.
Test Plan: just run make check
Reviewers: raster, cedric
Subscribers: jpeg
Differential Revision: https://phab.enlightenment.org/D4545
this allows environment variables to set the byte falue to fill a
freeq item added to the queue and then another item to actually fill
memory with just before the free function so memory content difference
will tell you if its inside the free queue or already freed from it
completely. if you set tyhe freed value to 0 this will not fill with a
value just before free and leave the value as-is determined by the
first fill pattern value.
Summary:
the api returns if a rectangle is positioned above/below/right or left
of a other rectangle.
Code which does simular things are part of verne and e, i think its a good idea to provide api for that.
Test Plan: Just run the test suite
Reviewers: raster, jpeg, cedric
Reviewed By: cedric
Differential Revision: https://phab.enlightenment.org/D4489
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
Some code needs to read directly into eina_binbuf to avoid an extra
copy from eina_binbuf_append* variants.
This can be achieved by using eina_binbuf_expand(), which returns a
write-able slice of the spare bytes. Once they are used,
eina_binbuf_use() should be called to increment "buf->len", which is
used by all other binbuf functions such as eina_binbuf_length_get() or
eina_binbuf_append_slice().
file != NULL does not mean it's valid. Since Eina_File is
a basic eina type a magic check is still better than nothing.
It can avoid doing eina_file_dup() on a closed file for instance.
This "fixes" a crash in eina_file_close with invalid files.
Now I can go hunt the root cause...
In C we need this to make clear that we really do not accept parameters.
Found by the smatch source code matcher. I had run and fixed this before
but it seems to creep in again over time.
Brought up by running smatch. We have way to many of such things in tree though
to fix them all without annoying a lot of people. I will just stop here.
Summary:
Since eina_model was dropped some years ago.
Also a few other points where related stuff is just commented out.
Reviewers: iscaro, barbieri
Reviewed By: barbieri
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4442
@fix
Summary: add ARG_NONNULL to eina_log* APIs for Eina_Log_Domain * parameter that is always in use, can not be NULL.
Reviewers: cedric, Hermet, myoungwoon, NikaWhite
Reviewed By: NikaWhite
Subscribers: t.naumenko, jpeg
Differential Revision: https://phab.enlightenment.org/D4426
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
Func _eina_file_win32_first_file try to find the first file in directory
but if any file not found the file handler stay open, and func will
return error. But in this case while handle is open impossible to do
any actions. For example call eina_file_ls for empty folder, func will
return error and fold folder open. And if we try to remove this folder
Windows only mark it to delete, and remove it after the process is
complete.
Solution: close handler in error case.
Summary:
fix warnings while generating documents
- end of file while inside a group (eina_util.h)
- missing title after \defgroup
- ignoring title "Ecore_Con_Lib_Group" that does not match old title
Reviewers: Hermet
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4420
Summary:
the new iterator represents the order from the elements of the original
iterator, elements where the filter callback return false will be
skipped.
The container of this iterator is the original iterator.
Test Plan: Just run `make check` there is a testcase
Reviewers: cedric, jpeg, raster, herdsman
Differential Revision: https://phab.enlightenment.org/D4417
Seems to me there is little benefit of inlining this function, but this
also had a pervert effect on Windows and C++ with some recent mingw
versions. Mingw failed its implementation of pthread_cleanup_pop(). It
does not compile when compiled in C++. There is a type mismatch that is
caught by the compiler, and everything goes nuts...
This made the EFL build fail because some files of ecore_win32 are C++
sources, and they require Eina... so this macro appears in a C++ code
indirectly, because of its inlining.
By removing the inlining, this build issue is fixed. Will also fix
builds of other programs that would have used Eina.h in their C++
programs :)
this will make a freeq bypass that is enabled by using valgrind or env
var not affect a freeq that has manually changed its queue count max
or mem max. these now become explicit deferred freeers.
this checks for clock_gettime + CLOCK_MONOTONIC or CLOCK_REALTIME at
evlog init to avoid a cmp+brang and l1 instr cache hit every get.
slightly less overhead when this is on.
this runs a 1000hz (or as best the kernel will allow) polling system
monitor thread that will logg the cpu frequencies of all cores (linux
only) as well as cpu usage per thread. this leads to much more
information able to be logged from an efl app (any efl app).
@feature
Summary: Add checking on NULL like in other API in module, to avoid segmentation fault
Reviewers: NikaWhite, cedric
Reviewed By: cedric
Subscribers: myoungwoon, jpeg
Differential Revision: https://phab.enlightenment.org/D4383
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
so i have been doing some profiling on my rpi3 ... and it seems
memcmp() is like the number one top used function - especially running
e in wayland compositor mode. it uses accoring to perf top about 9-15%
of samples (samples are not adding up to 100%). no - i cant seem to
get a call graph because all that happens is the whole kernel locks up
solid if i try, so i can only get the leaf node call stats. what
function was currently active at the sample time. memcmp is the
biggest by far. 2-3 times anything else.
13.47% libarmmem.so [.] memcmp
6.43% libevas.so.1.18.99 [.] _evas_render_phase1_object_pro
4.74% libevas.so.1.18.99 [.] evas_render_updates_internal.c
2.84% libeo.so.1.18.99 [.] _eo_obj_pointer_get
2.49% libevas.so.1.18.99 [.] evas_render_updates_internal_l
2.03% libpthread-2.24.so [.] pthread_getspecific
1.61% libeo.so.1.18.99 [.] efl_data_scope_get
1.60% libevas.so.1.18.99 [.] _evas_event_object_list_raw_in
1.54% libevas.so.1.18.99 [.] evas_object_smart_changed_get
1.32% libgcc_s.so.1 [.] __udivsi3
1.21% libevas.so.1.18.99 [.] evas_object_is_active
1.14% libc-2.24.so [.] malloc
0.96% libevas.so.1.18.99 [.] evas_render_mapped
0.85% libeo.so.1.18.99 [.] efl_isa
yeah. it's perf. it's sampling so not 100% accurate, but close to
"good enough" for the bigger stuff. so interestingly memcmp() is
actually in a special library/module (libarmmem.so) and is a REAL
function call. so doing memcmp's for small bits of memory ESPECIALLY
when we know their size in advance is not great. i am not sure our own
use of memcmp() is the actual culprit because even with this patch
memcmp still is right up there. we use it for stringshare which is
harder to remove as stringshare has variable sized memory blobs to
compare.
but the point remains - memcmp() is an ACTUAL function call. even on
x86 (i checked the assembly). and replacing it with a static inline
custom comparer is better. in fact i did that and benchmarked it as a
sample case for eina_tiler which has 4 ints (16 bytes) to compare
every time. i also compiled to assembly on x86 to inspect and make sure
things made sense.
the text color compare was just comparing 4 bytes as a color (an int
worth) which was silly to use memcmp on as it could just cast to an
int and do a == b. the map was a little more evil as it was 2 ptrs
plus 2 bitfields, but the way bitfields work means i can assume the
last byte is both bitfields combined. i can be a little more evil for
the rect tests as 4 ints compared is the same as comparing 2 long
longs (64bit types). yes. don't get pedantic. all platforms efl works
on work this way and this is a base assumption in efl and it's true
everywhere worth talking about.
yes - i tried __int128 too. it was not faster on x86 anyway and can't
compile on armv7. in my speed tests on x86-64, comparing 2 rects by
casting to a struct of 2 long long's and comparing just those is 70%
faster than comapring 4 ints. and the 2 long longs is 360% faster than
a memcmp. on arm (my rpi3) the long long is 12% faster than the 4 ints,
and it is 226% faster than a memcmp().
it'd be best if we didnt even have to compare at all, but with these
algorithms we do, so doing it faster is better.
we probably should nuke all the memcmp's we have that are not of large
bits of memory or variable sized bits of memory.
i set breakpoints for memcmp and found at least a chunk in efl. but
also it seems the vc4 driver was also doing it too. i have no idea how
much memory it was doing this to and it may ultimately be the biggest
culprit here, BUT we may as well reduce our overhead since i've found
this anyway. less "false positives" when hunting problems.
why am i doing this? i'm setting framerate hiccups. eg like we drop 3,
5 or 10 frames, then drop another bunch, then go back to smooth, then
this hiccup again. finding out WHAT is causing that hiccup is hard. i
can only SEE the hiccups on my rpi3 - not on x86. i am not so sure
it's cpufreq bouncing about as i've locked cpu to 600mhz and it still
happens. it's something else. maybe something we are polling? maybe
it's something in our drm/kms backend? maybe its in the vc4 drivers or
kernel parts? i have no idea. trying to hunt this is hard, but this is
important as this is something that possibly is affecting everyone but
other hw is fast enough to hide it...
in the meantime find and optimize what i find along the way.
@optimize
this adds eina_freeq api's for c land for deferring freeing of
pointers and can be used a s a simple copy & paste drop-in for free()
just to "do this later". the pointer will eveentually be freed as
eina_shutdown will free the main free queue and this will in turn free
everything in it. as long as the main lo0op keeps pumping things will
og on the queue and then be freed from it. free queues have limits so
if they get full they will clear out old pointers and free them so it
won't grow without bound. the default max is 1mb of data or 16384
items whichever limit is hit first and at that point the oldest item
will be freed to make room for the newest. the mainloop whenever it
finishes idle enterers will add an idler to spin and free while idle.
the sizes can be tuned and aruged about as to what defaults should be.
this also allows for better memory debugging too by being able to fill
freed memory with patterns if its small enough etc. etc.
@feature
following my commit for ecore_init(), do that for eina_init() as well,
sometimes it's hard to find the vars and their meaning without looking
at the code.
in eina_file we are using eina_hash, eina_hash is using eina_rbtree, so
we should ensure that rbtree is shutted down AFTER file is shutted down.
fix T4753
It's not because the bug with __builtin_prefetch is inside
clang/llvm that we must break the build for people who prefer it
over gcc. As soon as a non-broken version is out, the ifdef must
be either removed (and ask people to update their clang install)
or add a version check based on __clang_xxx__.
Compilation tested with clang 3.8.1 and gcc 6.2.1.
Andy Williams
Carsten Haitzler
Mike Blumenkrantz
Stefan Schmidt
Al Poole
Jean-Philippe Andre
Jean Guyomarc'h
Daniel Zaoui
Cedric BAIL
Jérémy Zurcher
Jeeyong Um
Prasoon Singh
Ivan Furs
Gustavo Sverzut Barbieri
Vincent Torri
Artem Popov
Marcel Hollerbach
Chris Michael
Flavio Ceolin
Myoungwoon Roy, Kim
Mykyta Biliavskyi
Derek Foreman
Jee-Yong Um
Bruno Dilly
Vyacheslav Reutskiy