Patch looks fine but I really prefer sizeof (type) vs sizeof type (ie
use parens around sizeof types).  Could you make this change?

Regards
-steve


On Fri, 2009-04-03 at 16:39 +0200, Jim Meyering wrote:
> Here's another buflen-adding patch, to give you an idea of what's in the
> pipe.  For the moment, I'm mainly in the mode of identifying and marking
> API changes, but not pulling each thread to propagate them throughout
> the code base.  However, for this one, since it's not too big, I have
> done that, to get an idea of how invasive such a change will end up being.
> 
> There haven't been very many of these parameter-adding changes so far
> (maybe a handful), but I have identified on the order of 100 int->size_t
> or const-related interface changes.  I'm at the 25-30% mark,
> in auditing the .h files under corosync/include.
> 
> I haven't tested at all, but the result does compile just as
> well as the original.
> 
> From 61595f5b4711ca97a9461992fbc33d8704c0e60c Mon Sep 17 00:00:00 2001
> From: Jim Meyering <[email protected]>
> Date: Fri, 3 Apr 2009 15:45:23 +0200
> Subject: [PATCH] coroipcc. h (coroipcc_dispatch_recv): Add a buflen parameter.
> 
> * lib/coroipcc.c (coroipcc_dispatch_recv): Update definition, and...
> (memcpy_swrap): ... add a parameter here, too.
> * include/corosync/coroipcc.h (coroipcc_dispatch_recv):
> * lib/cfg.c (corosync_cfg_dispatch):
> * lib/confdb.c (confdb_dispatch):
> * lib/cpg.c (cpg_dispatch, cpg_flow_control_state_get):
> * lib/evs.c (evs_dispatch):
> * lib/quorum.c (quorum_dispatch):
> * lib/votequorum.c (votequorum_dispatch):
> ---
>  include/corosync/coroipcc.h |    3 ++-
>  lib/cfg.c                   |    4 +++-
>  lib/confdb.c                |    4 +++-
>  lib/coroipcc.c              |   17 +++++++++++------
>  lib/cpg.c                   |    6 ++++--
>  lib/evs.c                   |    7 +++++--
>  lib/quorum.c                |    6 ++++--
>  lib/votequorum.c            |    8 ++++----
>  8 files changed, 36 insertions(+), 19 deletions(-)
> 
> diff --git a/include/corosync/coroipcc.h b/include/corosync/coroipcc.h
> index 1bf602a..81802ba 100644
> --- a/include/corosync/coroipcc.h
> +++ b/include/corosync/coroipcc.h
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright (c) 2002-2003 MontaVista Software, Inc.
> - * Copyright (c) 2006-2007 Red Hat, Inc.
> + * Copyright (c) 2006-2007, 2009 Red Hat, Inc.
>   *
>   * All rights reserved.
>   *
> @@ -85,6 +85,7 @@ int
>  coroipcc_dispatch_recv (
>       void *ipc_context,
>       void *buf,
> +     size_t buflen,
>       int timeout);
> 
>  int
> diff --git a/lib/cfg.c b/lib/cfg.c
> index 326c5dc..0d3ab34 100644
> --- a/lib/cfg.c
> +++ b/lib/cfg.c
> @@ -195,7 +195,9 @@ corosync_cfg_dispatch (
> 
>       do {
>               dispatch_avail = coroipcc_dispatch_recv (cfg_instance->ipc_ctx,
> -                     (void *)&dispatch_data, timeout);
> +                                                      (void *)&dispatch_data,
> +                                                      sizeof dispatch_data,
> +                                                      timeout);
> 
>               /*
>                * Handle has been finalized in another thread
> diff --git a/lib/confdb.c b/lib/confdb.c
> index 8311221..8f76a67 100644
> --- a/lib/confdb.c
> +++ b/lib/confdb.c
> @@ -330,7 +330,9 @@ cs_error_t confdb_dispatch (
>               pthread_mutex_lock (&confdb_inst->dispatch_mutex);
> 
>               dispatch_avail = coroipcc_dispatch_recv (confdb_inst->ipc_ctx,
> -                     (void *)&dispatch_data, timeout);
> +                                                      (void *)&dispatch_data,
> +                                                      sizeof dispatch_data,
> +                                                      timeout);
> 
> 
>               /*
> diff --git a/lib/coroipcc.c b/lib/coroipcc.c
> index ad60c73..c7ac176 100644
> --- a/lib/coroipcc.c
> +++ b/lib/coroipcc.c
> @@ -440,8 +440,8 @@ coroipcc_fd_get (void *ipc_ctx)
>       return (ipc_segment->fd);
>  }
> 
> -static void memcpy_swrap (
> -     void *dest, void *src, int len, unsigned int *read)
> +static void memcpy_swrap (void *dest, size_t dest_len,
> +                       void *src, int len, unsigned int *read)
>  {
>       char *dest_chr = (char *)dest;
>       char *src_chr = (char *)src;
> @@ -466,7 +466,7 @@ static void memcpy_swrap (
>  int original_flow = -1;
> 
>  int
> -coroipcc_dispatch_recv (void *ipc_ctx, void *data, int timeout)
> +coroipcc_dispatch_recv (void *ipc_ctx, void *data, size_t buflen, int 
> timeout)
>  {
>       struct pollfd ufds;
>       struct sembuf sop;
> @@ -546,23 +546,28 @@ retry_semop:
>       if (res == -1) {
>               return (-1);
>       }
> -     
> +
> +     if (buflen < DISPATCH_SIZE) {
> +             return -1;
> +     }
> +
>       if (ipc_segment->shared_memory->read + sizeof (mar_res_header_t) >= 
> DISPATCH_SIZE) {
>               my_read = ipc_segment->shared_memory->read;
> -             memcpy_swrap (data,
> +             memcpy_swrap (data, DISPATCH_SIZE,
>                       ipc_segment->shared_memory->dispatch_buffer,
>                       sizeof (mar_res_header_t),
>                       &ipc_segment->shared_memory->read);
>               header = (mar_res_header_t *)data;
>               memcpy_swrap (
>                       (void *)((char *)data + sizeof (mar_res_header_t)),
> +                     DISPATCH_SIZE,
>                       ipc_segment->shared_memory->dispatch_buffer,
>                       header->size - sizeof (mar_res_header_t),
>                       &ipc_segment->shared_memory->read);
>       } else {
>               header = (mar_res_header_t 
> *)&ipc_segment->shared_memory->dispatch_buffer[ipc_segment->shared_memory->read];
>               memcpy_swrap (
> -                     data,
> +                     data, DISPATCH_SIZE,
>                       ipc_segment->shared_memory->dispatch_buffer,
>                       header->size,
>                       &ipc_segment->shared_memory->read);
> diff --git a/lib/cpg.c b/lib/cpg.c
> index 1d05e64..5215f2d 100644
> --- a/lib/cpg.c
> +++ b/lib/cpg.c
> @@ -266,7 +266,9 @@ cs_error_t cpg_dispatch (
>               pthread_mutex_lock (&cpg_inst->dispatch_mutex);
> 
>               dispatch_avail = coroipcc_dispatch_recv (cpg_inst->ipc_ctx,
> -                     (void *)&dispatch_data, timeout);
> +                                                      (void *)&dispatch_data,
> +                                                      sizeof dispatch_data,
> +                                                      timeout);
> 
>               pthread_mutex_unlock (&cpg_inst->dispatch_mutex);
> 
> @@ -647,7 +649,7 @@ cs_error_t cpg_flow_control_state_get (
>       if (error != CS_OK) {
>               return (error);
>       }
> -     
> +
>       *flow_control_state = coroipcc_dispatch_flow_control_get 
> (cpg_inst->ipc_ctx);
> 
>       saHandleInstancePut (&cpg_handle_t_db, handle);
> diff --git a/lib/evs.c b/lib/evs.c
> index 13936fb..0b04c10 100644
> --- a/lib/evs.c
> +++ b/lib/evs.c
> @@ -2,7 +2,7 @@
>   * vi: set autoindent tabstop=4 shiftwidth=4 :
> 
>   * Copyright (c) 2004-2005 MontaVista Software, Inc.
> - * Copyright (c) 2006-2007 Red Hat, Inc.
> + * Copyright (c) 2006-2007, 2009 Red Hat, Inc.
>   *
>   * All rights reserved.
>   *
> @@ -222,7 +222,10 @@ evs_error_t evs_dispatch (
>       }
> 
>       do {
> -             dispatch_avail = coroipcc_dispatch_recv (evs_inst->ipc_ctx, 
> (void *)&dispatch_data, timeout);
> +             dispatch_avail = coroipcc_dispatch_recv (evs_inst->ipc_ctx,
> +                                                      (void *)&dispatch_data,
> +                                                      sizeof dispatch_data,
> +                                                      timeout);
>               if (dispatch_avail == -1) {
>                       error = CS_ERR_LIBRARY;
>                       goto error_nounlock;
> diff --git a/lib/quorum.c b/lib/quorum.c
> index df05876..a18ec83 100644
> --- a/lib/quorum.c
> +++ b/lib/quorum.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008 Red Hat, Inc.
> + * Copyright (c) 2008, 2009 Red Hat, Inc.
>   *
>   * All rights reserved.
>   *
> @@ -392,7 +392,9 @@ cs_error_t quorum_dispatch (
>               pthread_mutex_lock (&quorum_inst->dispatch_mutex);
> 
>               dispatch_avail = coroipcc_dispatch_recv (quorum_inst->ipc_ctx,
> -                     (void *)&dispatch_data, timeout);
> +                                                      (void *)&dispatch_data,
> +                                                      sizeof dispatch_data,
> +                                                      timeout);
> 
>               /*
>                * Handle has been finalized in another thread
> diff --git a/lib/votequorum.c b/lib/votequorum.c
> index 9922b31..fd71a23 100644
> --- a/lib/votequorum.c
> +++ b/lib/votequorum.c
> @@ -776,10 +776,10 @@ cs_error_t votequorum_dispatch (
>       do {
>               pthread_mutex_lock (&votequorum_inst->dispatch_mutex);
> 
> -             dispatch_avail = coroipcc_dispatch_recv (
> -                     votequorum_inst->ipc_ctx,
> -                     (void *)&dispatch_data, timeout);
> -
> +             dispatch_avail = coroipcc_dispatch_recv 
> (votequorum_inst->ipc_ctx,
> +                                                      (void *)&dispatch_data,
> +                                                      sizeof dispatch_data,
> +                                                      timeout);
> 
>               /*
>                * Handle has been finalized in another thread

_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais

Reply via email to