Open zackw opened 2 years ago
Hello team, Let me explain the steps to reproduce.
The example_cmdlib.c works fine. https://github.com/lvmteam/lvm2/blob/master/doc/example_cmdlib.c But, stdout will be corrupted when I define a function in example_cmdlib.c like this:
#include "tools/lvm2cmd.h"
#include <stdio.h>
#include <stdarg.h>
int my_function(const char *formatP, ...)
{
va_list ap;
va_start(ap, formatP);
vfprintf(stdout, formatP, ap);
va_end(ap);
return 0;
}
/* All output gets passed to this function line-by-line */
void test_log_fn(int level, const char *file, int line,
int dm_errno, const char *format)
{
/* Extract and process output here rather than printing it */
if (level != 4)
return;
printf("%s\n", format);
return;
}
int main(int argc, char **argv)
{
void *handle;
int r;
lvm2_log_fn(test_log_fn);
handle = lvm2_init();
lvm2_log_level(handle, 1);
r = lvm2_run(handle, "vgs");
/* More commands here */
lvm2_exit(handle);
return r;
}
The print( ) AIP in test_log_fn doesn't work when I define my_function.
./configure --enable-cmdlib # How I configure LVM
/lib/x86_64-linux-gnu/libc.so.6 # The libc in my running environment
GNU C Library (Ubuntu GLIBC 2.23-0ubuntu11.3) stable release version 2.23
The core problem was ensure what kind of buffering glibc will use - and since lvm2 works across numerous version of glibc - various versions seemed to have different problems with our major requirement to use already preallocated buffer (so glibc does not resize/reallocate it's internal stream buffers later while i.e. buffering log output while we do not want any allocation/mmap to happen.
So while the code might look weird we are not aware of any crashes/memory leak with current solution - if there is any - please report as bug with reproducer.
As we no longer support any library usage from lvm (user should stay with call of lvm command - eventually they may try D-Bus - although this API has its limits) - I'm not quite sure what is 2nd. comment about ??
This part of
reopen_standard_stream
mixes operations at the fd level with operations at the FILE level in a way that doesn't make any sense to me and, as reported (somewhat incoherently) at https://stackoverflow.com/questions/70879665/ , can wind up corrupting the C library's internal state associated withstdout
. Also, several of the error handling paths are incorrect and would cause either crashes or file descriptor leaks.To the extent I understand what this is trying to do, it looks like a corrected version would be
That is, don't try to close the original FILE object at all, just make a second FILE pointing to a duplicate of the original file descriptor, which the library can use internally as it sees fit.
It's possible that my suggestion doesn't accomplish what this code is supposed to do; if you could explain the actual goal I can maybe give better advice.