latchset / libverto Goto Github PK

Issue migrated from trac ticket # 11

component: component1 | priority: major | resolution: wontfix

2012-01-18 10:26:10: @simo5 created the issue

My program used to set up one persistent read event, read some data and pass it to be processed by a thread.
Once the data is back a new non persistent write event was created to deal with writing.
On write error the write event callback would free the read event and close the connection.

   1.
      Program received signal SIGABRT, Aborted.
   2.
      0x0000003910436285 in raise () from /lib64/libc.so.6
   3.
      (gdb) bt
   4.
      #0 0x0000003910436285 in raise () from /lib64/libc.so.6
   5.
      #1 0x0000003910437b9b in abort () from /lib64/libc.so.6
   6.
      #2 0x00000035ee6021e6 in ?? () from /usr/lib64/libtalloc.so.2
   7.
      #3 0x00000035ee6038be in _talloc_free () from /usr/lib64/libtalloc.so.2
   8.
      #4 0x00007ffff7dda46c in verto_del () from /usr/lib64/libverto.so.1
   9.
      #5 0x0000000000401edb in gp_conn_free (conn=0x60abb0) at src/gp_socket.c:78

Issue migrated from trac ticket # 10

component: component1 | priority: minor | resolution: fixed

2011-09-12 11:48:00: @greghudson created the issue

Symbols beginning with an underscore are reserved by the C89 standard for various purposes. verto uses a number of symbols like this and probably should not.

(In relation to this trace)

I've been trying to track down why filename is leaking from here. As far as I can see, loaded_modules itself is also never freed. In particular, gssproxy calls verto_default(...) with impl==NULL here. The resulting vctx is later free-d in a bunch of places, including here. However, it appears that loaded_modules is not touched as part of that call.

As far as I understand it, this could be desired behavior in case an application wants to open a second event loop (e.g., the test cases); thus it would appear an explicit verto_cleanup() call would be needed to free filename/loaded_modules.

Should this be added to verto_free(...), or a new verto_cleanup() call?

cc: @simo5 @frozencemetery

Issue migrated from trac ticket # 8

component: component1 | priority: minor | resolution: fixed

2011-09-08 11:41:36: @greghudson created the issue

In the krb5 tree, when we build a bundled libverto, we want to avoid the dynamic linking logic and just use the bundled k5ev module. Currently we take care of this in the calling code, which requires conditional behavior in three places:

1. Link against -lverto-k5ev -lverto instead of -lverto.
2. Include <verto-k5ev.h> instead of <verto.h>
3. Call verto_default_k5ev() instead of verto_default(NULL, types).

As we add more call sites, we can manage this caller complexity with a few macros. But as a matter of preference and style, I feel like it's more elegant to handle this under the covers.

My idea is to support a compiler symbol in verto.c with a name like VERTO_USE_MODULE, whose value is probably the module table symbol from the module library (which gets linked as a dependency library by whatever is specially building verto.c). If defined, the dynamic loading logic would be compiled out (improving portability) and load_module() just checks reqtypes against the module and returns it. I think this level of internal conditionalization shouldn't be any worse than the equivalent in the caller.

Issue migrated from trac ticket # 7

component: component1 | priority: major | resolution: fixed

2011-09-07 14:07:27: @greghudson created the issue

verto_set_private() can only return failure on a specific invalid input (a null ev pointer). So it is almost never necessary to check the return value, but static analyzers like lint will tend to object to code which ignores it. As a simple non-copying setter function, it would be better for verto_set_private() to return void.

(This is a matter of opinion and style, of course.)

Issue migrated from trac ticket # 12

component: component1 | priority: major | resolution: fixed

2012-05-13 13:51:33: @greghudson created the issue

verto_convert_module contains a sanity check for module being NULL. But if that check fires, it goes to the error label, which dereferences module if mctx is set.

(Found with scan-build.)

Issue migrated from trac ticket # 2

component: component1 | priority: major | resolution: fixed

2011-09-07 13:48:41: @greghudson created the issue

In the libev modules, libev_ctx_del() appears to be missing necessary break statements in its switch statement.

Currently it's disabled in the CI, but Debian also turns off building of the tevent backend entirely, presumably for this reason.

they currently install as:

prefix=/usr
libdir=${exec_prefix}/lib
includedir=${prefix}/include
exec_prefix=${prefix}

but this means that libdir references a nonexistent exec_prefix.
This in turn breaks compiling krb5, which does

pkg-config --print-errors --cflags --libs "libverto"
Variable 'exec_prefix' not defined in '/usr/lib/pkgconfig/libverto.pc'

Issue migrated from trac ticket # 3

component: component1 | priority: major | resolution: fixed

2011-09-07 13:52:39: @greghudson created the issue

libverto's headers contain a number of function declarations which aren't prototypes because they use () instead of (void). In definitions this is okay, but in prototypes this causes the compiler to treat the declaration as a K&R-style declaration and not a prototype.

verto-glib.h:verto_ctx *verto_new_glib();
verto-glib.h:verto_ctx *verto_default_glib();
verto-libevent.h:verto_ctx *verto_new_libevent();
verto-libevent.h:verto_ctx *verto_default_libevent();
verto-libev.h:verto_ctx *verto_new_libev();
verto-libev.h:verto_ctx *verto_default_libev();
verto-module.h:typedef verto_ctx *(*verto_ctx_constructor)();
verto-tevent.h:verto_ctx *verto_new_tevent();
verto-tevent.h:verto_ctx *verto_default_tevent();

When building libverto as a static library,
then verto_new returns NULL.

Building it as a shared library works though.

I ran into a problem detecting the system libev packaging libverto 0.3.0 for Debian.
The debian ev.h has code like:

#if __STDC_VERSION__ >= 199901L || __GNUC__ >= 3
# define EV_INLINE static inline
#else
# define EV_INLINE static
#endif
EV_INLINE struct ev_loop *
ev_default_loop_uc_ (void) EV_NOEXCEPT

And when building with -std=c89 I get errors like:

/usr/include/ev.h:564:11: error: expected ‘;’ before ‘struct’
 EV_INLINE struct ev_loop *
           ^~~~~~

I don't quite understand what's going wrong, but honestly I shouldn't have to. It's 2019; c99 is 20 years old.
It's unsurprising that even if verto tests c89 complience, not all dependencies will do so, and the problem will become worse not better over time.
It seems fine to run a CI build forcing c89 to make sure it works if you care about that, but I think for people building libev to actually use the software, requiring all dependencies build in c89 mode is overly fragile.

Issue migrated from trac ticket # 4

component: component1 | priority: major | resolution: fixed

2011-09-07 13:58:15: @greghudson created the issue

verto.c:load_module() assumes that it can transform the pathname of the libverto library into the pathname of a module library by adding "-modulename" just before the first occurrence of ".so" in the pathname. This assumption breaks down on non-ELF platforms. On OSX, for instance, the library suffix is .dylib, and the version number precedes the suffix (libverto.0.dylib).

An alternative might be using the last occurrence of "verto" in the pathname as the breakpoint.

Issue migrated from trac ticket # 1

component: component1 | priority: major | resolution: fixed

2011-09-07 13:48:02: @greghudson created the issue

verto uses declarations after statements in multiple places. This is a C99 feature and doesn't work in MSVC. Compiling with -Wdeclaration-after-statement will detect these uses.

Issue migrated from trac ticket # 6

component: component1 | priority: major | resolution: fixed

2011-09-07 14:02:06: @greghudson created the issue

libverto's installed pkg-config data files are broken in most builds because they come out as:

prefix=/usr/local
libdir=${exec_prefix}/lib
includedir=${prefix}/include

but never define exec_prefix. They all need an exec_prefix=@exec_prefix@ line.

Issue migrated from trac ticket # 9

component: component1 | priority: minor | resolution: fixed

2011-09-08 11:50:44: @greghudson created the issue

verto has a dependency on -ldl on most platforms, which comes from the @libs@. According to current documentation, the right thing to do is have a line in libverto.pc.in saying:

Libs.private: @libs@

Dependencies given this way will only be emitted for static linking. For shared linking, you don't want the consuming library or program to have a direct dependency on libdl.

Issue migrated from trac ticket # 5

component: component1 | priority: major | resolution: fixed

2011-09-07 14:00:25: @greghudson created the issue

libverto modules can be used as dynamically loaded objects or as directly linked libraries. They export module-specific symbols like verto_libev_default(), but also a fixed-named data symbol verto_module_table. Trying to link against two module libraries (perhaps one by the application and another by a subsidiary library which uses a private event loop) would result in a symbol conflict.

Verbally, Nathaniel has suggested that the module table symbol name will be changed to include the module name. This will require deducing the module name from the pathname in verto.c:do_load_dir().

There is a high performance event library called libhv.
It has faster implementation about event loop functions, see https://github.com/ithewei/libhv#-benchmark

libverto has encapsulated libev, libevent as backend, suggest to include libhv as well.
if it is okay to introduce libhv into libverto, I will try to submit a commit.

Issue migrated from trac ticket # 13

component: component1 | priority: major

2015-07-15 16:53:02: @tkuthan created the issue

verto_set_allocator(resize, hierarchical) has an unducumented
requirement on allocator function resize, that resize(ptr, 0) is
equivalent to free(ptr).

Some POSIX.1-2008 compliant implementations of the default allocator realloc don't
meet this requirement. realloc(ptr, 0) may return a unique pointer that can be
successfully passed to free. This new pointer never gets freed by libverto and the
memory overhead leaks.

This is not an issue for linux realloc implementation, but libverto has been bundled with MIT Kerberos. MIT Kerberos is supported on platforms like Oracle Solaris, where realloc(ptr, 0) is not equivalent to free(ptr).

See krb5/krb5#294