Hi Oliver,
[email protected] wrote:
> I have done several changes in rt_print.c and rtdk.h to extend
> the functionality of rt-print lib as follows :
> - Fork save initialisation of the library by using pthread_atfork()
> There are now two hooks integrated :
> 1. A prepare hook which is called to prepare the fork. This
> hook clears all open print buffers
> 2. A child hook which is called after the fork on the child side.
> This hook clears also all open print buffers (just ot be on the save
> side)
> and is spawning another printer thread.
>
> - Adding two addition functions
> 1. rt_syslog( int priority, char *format, ...)
> which reports all messages to the syslog, using the print buffers of
> rtprint
> 2. rt_vsyslog( int priority, char *format, va_list args )
> same as above for va_list arguments.
>
> Both changes do run in my application as expected. It's now possible to use
> rt_print-lib in a daemonized process and rt_print output can be directed
> to syslog via rt_syslog/rt_vsyslog.
Thanks for the patch. I just have a few comments, see below.
>
>
> Oliver
>
>
>
> --- rtdk.h.original 2007-12-09 11:46:36.000000000 +0100
> +++ rtdk.h 2009-10-09 11:46:52.767271100 +0200
>
> int rt_vprintf(const char *format, va_list args);
> int rt_fprintf(FILE *stream, const char *format, ...);
> int rt_printf(const char *format, ...);
> +void rt_syslog( int priority, char *format, ...);
^^^
Stray whitespace.
> +void rt_vsyslog(int priority, char *format, va_list args);
> +
>
> int rt_print_init(size_t buffer_size, const char *name);
> void rt_print_cleanup(void);
>
>
> --- rt_print.c.original 2008-09-10 10:36:27.000000000 +0200
> +++ rt_print.c 2009-10-09 11:47:16.014443900 +0200
>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> +#include <syslog.h>
>
> #include <rtdk.h>
> #include <asm/xenomai/system.h>
>
>
> #define RT_PRINT_LINE_BREAK 256
>
> +#define RT_PRINT_SYSLOG_STREAM ((FILE *)-9999)
Better use NULL here.
> +
> struct entry_head {
> FILE *dest;
> uint32_t seq_no;
> + int priority;
> char text[1];
> } __attribute__((packed));
>
>
>
> static void cleanup_buffer(struct print_buffer *buffer);
> static void print_buffers(void);
> +static void rt_print_reinit_atfork_prepare(void);
> +static void rt_print_reinit_atfork_child(void);
>
> /* *** rt_print API *** */
>
> -int rt_vfprintf(FILE *stream, const char *format, va_list args)
> +static int rt_print_to_buffer(FILE *stream, int priority, const char
> *format, va_list args)
Line with >80 characters.
> {
> struct print_buffer *buffer = pthread_getspecific(__buffer_key);
> off_t write_pos, read_pos;
>
> /* Write out empty entry */
> head = buffer->ring + write_pos;
> head->seq_no = __seq_no;
> + head->priority = 0;
> head->text[0] = 0;
>
> /* Forward to the ring buffer start */
>
> /* If we were able to write some text, finalise the entry */
> if (len > 0) {
> head->seq_no = ++__seq_no;
> + head->priority = priority;
> head->dest = stream;
>
> /* Move forward by text and head length */
>
> /* An empty entry marks the wrap-around */
> head = buffer->ring + write_pos;
> head->seq_no = __seq_no;
> + head->priority = priority;
> head->text[0] = 0;
>
> write_pos = 0;
>
> return res;
> }
>
> +int rt_vfprintf(FILE *stream, const char *format, va_list args)
> +{
> + return rt_print_to_buffer( stream, 0, format, args);
Stray whitespace.
> +}
> +
> int rt_vprintf(const char *format, va_list args)
> {
> return rt_vfprintf(stdout, format, args);
>
> return n;
> }
>
> +void rt_syslog(int priority, char *format, ...)
> +{
> + va_list args;
> +
> + va_start(args, format);
> + rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
> + va_end(args);
> +
> + return;
> +}
> +
> +void rt_vsyslog(int priority, char *format, va_list args )
Whitespace.
> +{
> +
> + rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
> +
> + return;
> +}
> +
> static void set_buffer_name(struct print_buffer *buffer, const char *name)
> {
> int n;
>
>
> if (len) {
> /* Print out non-empty entry and proceed */
> - fprintf(head->dest, "%s", head->text);
> + /* Check if output goes to syslog */
> + if (head->dest == RT_PRINT_SYSLOG_STREAM) {
> + syslog( head->priority, "%s", head->text );
Stray whitespaces. And one /may/ argue that we do not need braces here,
but that's a grey area.
> + } else {
> + /* Output goes to specified stream */
> + fprintf(head->dest, "%s", head->text);
> + }
> +
> read_pos += sizeof(*head) + len;
> } else {
> /* Emptry entries mark the wrap-around */
>
> }
> }
>
> +static void rt_print_reinit_atfork_child(void)
> +{
> + pthread_attr_t thattr;
> + struct print_buffer *buffer,*next_b;
> +
> + /* Clean parent process buffer list -
> + Shouldn't be necessary because also done at atfork_parent
> + */
> + buffer = __first_buffer;
> + while (buffer) {
> + next_b = buffer->next;
> + cleanup_buffer(buffer);
> + buffer = next_b;
> + }
> +
> + /* Throw new thread with printer loop in child process */
> + pthread_attr_init(&thattr);
> + pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> + pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +
> pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
> +}
Here is my version:
static void spawn_printer_thread(void)
{
pthread_attr_t thattr;
pthread_attr_init(&thattr);
pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
pthread_create(&printer_thread, &thattr, printer_loop, NULL);
}
static void forked_child_init(void)
{
struct print_buffer *my_buffer = pthread_getspecific(buffer_key);
struct print_buffer **pbuffer = &first_buffer;
/* re-init to avoid finding it locked by some parent thread */
pthread_mutex_init(&buffer_lock, NULL);
while (*pbuffer) {
if (*pbuffer == my_buffer)
pbuffer = &(*pbuffer)->next;
else
cleanup_buffer(*pbuffer);
}
spawn_printer_thread();
}
The major differences are:
- my code is totally untested :)
- we have to play safe and re-init the mutex
- we should not purge a potentially existing buffer of the main thread
- I don't think we have to re-install the fork hook for the child (the
parent's installation should survive the fork)
and...
> +
> +static void rt_print_reinit_atfork_prepare(void)
> +{
> + struct print_buffer *buffer,*next_b;
> +
> + /* Clean parent process buffer list */
> + buffer = __first_buffer;
> + while (buffer) {
> + next_b = buffer->next;
> + cleanup_buffer(buffer);
> + buffer = next_b;
> + }
> +}
> +
- we must not destroy the father's environment
> void __rt_print_init(void)
> {
> pthread_attr_t thattr;
>
> pthread_attr_init(&thattr);
> pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +
> pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
> }
>
Besides that, nice patch!
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
_______________________________________________
Xenomai-help mailing list
[email protected]
https://mail.gna.org/listinfo/xenomai-help
