[syslog-ng] [PATCH 1/3] scratch-buffers: Scratch buffer implementation.

[Balazs Scheidler]

On Mon, 2011-10-31 at 12:25 +0100, Gergely Nagy wrote:
> Scratch buffers are thread-local GString buffer stacks. The intended
> usage is that whenever a piece of code needs a scratch buffer for some
> quick task, it can acquire one via scratch_buffer_acquire(), and
> whenever it is not needed anymore, it releases it back to the stack.
> 
> Buffers are allocated on-demand, and they're freed up at the end of
> main_loop_io_worker_job_start(), when all other callbacks have
> finished already.
> 
> Signed-off-by: Gergely Nagy <algernon at balabit.hu>
> ---
>  lib/Makefile.am       |    2 +
>  lib/mainloop.c        |    2 +
>  lib/scratch-buffers.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/scratch-buffers.h |   41 +++++++++++++++++++++++++++++++
>  4 files changed, 110 insertions(+), 0 deletions(-)
>  create mode 100644 lib/scratch-buffers.c
>  create mode 100644 lib/scratch-buffers.h
> 
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index c4e42b7..7faf241 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -65,6 +65,7 @@ pkginclude_HEADERS = 		\
>  	plugin.h		\
>  	pragma-parser.h		\
>  	rewrite-expr-parser.h	\
> +	scratch-buffers.h	\
>  	serialize.h		\
>  	sgroup.h		\
>  	stats.h			\
> @@ -134,6 +135,7 @@ libsyslog_ng_la_SOURCES = \
>  	plugin.c		\
>  	pragma-parser.c		\
>  	rewrite-expr-parser.c	\
> +	scratch-buffers.c	\
>  	serialize.c		\
>  	sgroup.c		\
>  	stats.c			\
> diff --git a/lib/mainloop.c b/lib/mainloop.c
> index 92433b7..9b7d1f2 100644
> --- a/lib/mainloop.c
> +++ b/lib/mainloop.c
> @@ -32,6 +32,7 @@
>  #include "logqueue.h"
>  #include "dnscache.h"
>  #include "tls-support.h"
> +#include "scratch-buffers.h"
>  
>  #include <sys/types.h>
>  #include <sys/wait.h>
> @@ -364,6 +365,7 @@ main_loop_io_worker_job_start(MainLoopIOWorkerJob *self)
>        cb->func(cb->user_data);
>        list_del_init(&cb->list);
>      }
> +  scratch_buffers_reset ();
>    g_assert(list_empty(&self->finish_callbacks));
>    main_loop_current_job = NULL;
>  }
> diff --git a/lib/scratch-buffers.c b/lib/scratch-buffers.c
> new file mode 100644
> index 0000000..b254dae
> --- /dev/null
> +++ b/lib/scratch-buffers.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (c) 2011 BalaBit IT Ltd, Budapest, Hungary
> + * Copyright (c) 2011 Gergely Nagy <algernon at balabit.hu>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + * As an additional exemption you are allowed to compile & link against the
> + * OpenSSL libraries as published by the OpenSSL project. See the file
> + * COPYING for details.
> + *
> + */
> +
> +#include "tls-support.h"
> +#include "scratch-buffers.h"
> +
> +TLS_BLOCK_START
> +{
> +  GTrashStack *scratch_buffers;
> +}
> +TLS_BLOCK_END;
> +
> +#define local_scratch_buffers	__tls_deref(scratch_buffers)
> +
> +ScratchBuffer *scratch_buffer_acquire (void)
> +{
> +  ScratchBuffer *sb;
> +
> +  sb = g_trash_stack_pop (&local_scratch_buffers);
> +  if (!sb)
> +    {
> +      sb = g_new (ScratchBuffer, 1);
> +      sb->s = g_string_new (NULL);
> +    }
> +  else
> +    g_string_set_size (sb->s, 0);
> +  return sb;
> +}
> +
> +void scratch_buffer_release (ScratchBuffer *sb)
> +{
> +  g_trash_stack_push (&local_scratch_buffers, sb);
> +}
> +
> +void scratch_buffers_reset (void)
> +{
> +  ScratchBuffer *sb;
> +
> +  while ((sb = g_trash_stack_pop (&local_scratch_buffers)) != NULL)
> +    {
> +      g_string_free (sb->s, TRUE);
> +      g_free (sb);
> +    }
> +}

Please see my comments in the other email, although now I see how the
first 8 bytes of the GString instance are not clobbered :)

Also, reset should be called _before_ calling the job function, and the
full cleanup should be done once the thread finishes:
main_loop_io_worker_thread_stop()

Otherwise it looks nice.

One whitespace nitpick that I'd care about is to start the name of the
function on character 1, this makes it very easy to grep for the
definition of the function.

The others (using a space in front of '(') I don't care about as long as
it's consistent within the same file. Since these are new files, they
can be left as is.

-- 
Bazsi