Menu
▾
▴
libdbi-devel
|
From: Rainer G. <rge...@hq...> - 2008-02-16 13:51:53
Attachments:
libdbi-instances
|
As discussed on the -user mailing list, I have created a small patch to support calling libdbi from plugins (where several plugins may call into dbi_initialize() not knowing about each other). I have used the _r naming conventions for the changed functions. Please note that I needed to change a bit more code as rootconn was also affected by the requirement. I have tried to keep as consistent with the libdbi interface as possible. However, in dbi_initialize_r(), I have slightly different calling conventions. I now needed to pass a second output parameter (the instance pointer). Fully in line with existing calling conventions would be to make dbi_initialize_r() return the dbi_inst handle. However, that would have required to introduce a new function to query the loaded driver count. I think this would be more intrusive and probably confusing to existing code. So I decided to go for a output param. Please review the patch, comments are of course welcome. Rainer |
|
From: Markus H. <mar...@mh...> - 2008-02-16 16:10:23
|
Rainer Gerhards writes:
> Please review the patch, comments are of course welcome.
>
Thanks for the patch, which looks pretty clean and useful. However, a
quick test on FreeBSD using libdbi-drivers "make check" (which
still uses the *old* interface) causes a coredump:
5 drivers available: /usr/local/bin/bash: line 4: 5147 Segmentation fault: 11 (core dumped) ${dir}$tst
and gdb says:
(gdb) bt
#0 0x28080561 in dbi_driver_get_name () from /usr/local/lib/libdbi.so.0
#1 0x0804a8df in ask_for_conninfo (ptr_cinfo=0xbfbfe240) at test_dbi.c:525
#2 0x08049883 in main (argc=1, argv=0xbfbfe758) at test_dbi.c:69
I'll further investigate, but if you see an obvious error in the code
based on this observation, let me know.
regards,
Markus
--
Markus Hoenicka
mar...@ca...
(Spam-protected email: replace the quadrupeds with "mhoenicka")
http://www.mhoenicka.de
|
|
From: Rainer G. <rge...@hq...> - 2008-02-16 17:10:52
Attachments:
libdbi-instances-2
|
The problem was that I originally had the caller define the instance
structure (well, kind of... ;)). I discovered this didn't work (I was
too much rsyslog-minded, especially in my assumptions), changed things
and forgot to re-test with the old interface. New patch attached,
replaces the old one. I have done some limited testing with rsyslog
itself.
As a side-note, one must be careful when converting to the new
interface. If just the dbi_initialize_r() is used but the others use the
old functions, there is a lot of trouble. I ran myself into it.
Unfortunately, I have no smart idea how to circumvent it. The deprecated
macros will probably help... ;)
Rainer
On Sat, 2008-02-16 at 17:07 +0100, Markus Hoenicka wrote:
> Rainer Gerhards writes:
> > Please review the patch, comments are of course welcome.
> >
>
> Thanks for the patch, which looks pretty clean and useful. However, a
> quick test on FreeBSD using libdbi-drivers "make check" (which
> still uses the *old* interface) causes a coredump:
>
> 5 drivers available: /usr/local/bin/bash: line 4: 5147 Segmentation fault: 11 (core dumped) ${dir}$tst
>
> and gdb says:
>
> (gdb) bt
> #0 0x28080561 in dbi_driver_get_name () from /usr/local/lib/libdbi.so.0
> #1 0x0804a8df in ask_for_conninfo (ptr_cinfo=0xbfbfe240) at test_dbi.c:525
> #2 0x08049883 in main (argc=1, argv=0xbfbfe758) at test_dbi.c:69
>
> I'll further investigate, but if you see an obvious error in the code
> based on this observation, let me know.
>
> regards,
> Markus
>
|
|
From: Markus H. <mar...@mh...> - 2008-02-16 23:38:46
|
Rainer Gerhards writes: > The problem was that I originally had the caller define the instance > structure (well, kind of... ;)). I discovered this didn't work (I was > too much rsyslog-minded, especially in my assumptions), changed things > and forgot to re-test with the old interface. New patch attached, > replaces the old one. I have done some limited testing with rsyslog > itself. > This one works for me, using the old interface. I'll attempt to modify libdbi-driver's test program to allow testing both interfaces. I'll be back with the results of the new interface as soon as time permits. > As a side-note, one must be careful when converting to the new > interface. If just the dbi_initialize_r() is used but the others use the > old functions, there is a lot of trouble. I ran myself into it. > Unfortunately, I have no smart idea how to circumvent it. The deprecated > macros will probably help... ;) > A half-assed way to circumvent this problem is to have dbi_initialize_r set dbi_inst_legacy in addition to the structure pointed to by **Inst. This will allow to run the old functions safely and correctly as long as the initialization function is called only once. However, this way we'll probably help to cover up many incomplete migration attempts, so it may be more prudent to have the apps crash right away so they get fixed. regards, Markus -- Markus Hoenicka mar...@ca... (Spam-protected email: replace the quadrupeds with "mhoenicka") http://www.mhoenicka.de |
|
From: Rainer G. <rge...@hq...> - 2008-02-17 08:51:35
|
> > As a side-note, one must be careful when converting to the new > > interface. If just the dbi_initialize_r() is used but the others use > the > > old functions, there is a lot of trouble. I ran myself into it. > > Unfortunately, I have no smart idea how to circumvent it. The > deprecated > > macros will probably help... ;) > > > > A half-assed way to circumvent this problem is to have > dbi_initialize_r set dbi_inst_legacy in addition to the structure > pointed to by **Inst. This will allow to run the old functions safely > and correctly as long as the initialization function is called only > once. However, this way we'll probably help to cover up many > incomplete migration attempts, so it may be more prudent to have the > apps crash right away so they get fixed. I thought about that, but the "as long as the init function is called only once" is indication of a subtle problem with it. Consider this: - thinking about a plug-in system - still assume that the loader keeps a single static data area in memory no matter how many plugins use libdbi - there is *exactly* one plugin that uses the Old interface - there is one or more plugins using the new Interface In that situation, without the flag, everything works ok because the old interface plugin finds everything as expected. With a flag, the first call to dbi_initialize_r would kind of disable the old style interface calls, thus giving the old interface plugin a hard time. Granted, it is a very very unusual case, probably never to be seen in practice. It's also questionable if keeping things happy in that case outweights the benefits of another solution. But together with your thoughts on preventing incomplete migration attempts it probably adds to solid reasoning NOT to support such a flag. I just wanted to pass the info, hopefully its useful. Rainer |
|
From: Markus H. <mar...@mh...> - 2008-02-17 16:55:52
|
Rainer Gerhards writes: > The problem was that I originally had the caller define the instance > structure (well, kind of... ;)). I discovered this didn't work (I was > too much rsyslog-minded, especially in my assumptions), changed things > and forgot to re-test with the old interface. New patch attached, > replaces the old one. I have done some limited testing with rsyslog > itself. > I've applied and checked in the suggested patch with a few minor modifications: 1) dbi.h is an autogenerated file, so I moved the changes into dbi.h.in 2) some drivers need access to the instance (see below), so I added the accessor dbi_driver_get_instance() I've adapted test_dbi.c in libdbi-drivers and checked both interfaces. It turned out that some drivers (ingres, sqlite, sqlite3) run dbi_conn_new() in order to access databases other than the one they're connected to. This works when using the old interface, but must fail when the plugin-safe interface is used instead. I've changed these calls to dbi_conn_new_r() which works ok with both interfaces. However, this means that we'll have to move at least to 0.9 because the next libdbi release won't work with 0.8.x drivers as soon as you use the plugin-safe interface. Both libdbi and libdbi-drivers currently call themselves 0.9.0-pre1. Once we move to a new major release, we should review the discussion about unifying the version numbers and the library versioning information, see this feature request: http://sourceforge.net/tracker/index.php?func=detail&aid=1578577&group_id=65979&atid=512948 This would finally get us to 1.0 for both libdbi and libdbi-drivers. Before doing so, I'd once again ask everyone to test the new interface with all OSes you can get hold of, and to speak up now if any other important changes are required. I'll update the Programmer's Manual and the Driver Manual in the next days to reflect the API changes. regards, Markus -- Markus Hoenicka mar...@ca... (Spam-protected email: replace the quadrupeds with "mhoenicka") http://www.mhoenicka.de |
Thanks for helping keep SourceForge clean.
X