Re: [gpsd-dev] [PATCH 6/6] Eliminates the one use of the portability-challenged alloca().

[Gary E. Miller]

Yo Fred!

Patch applied.  Next time, please also add the patch as an attachment.

On Mon,  5 Sep 2016 18:09:28 -0700
Fred Wright <address@hidden> wrote:

> In general, the use of alloca() is discouraged.  Here, there was
> only one use, with a maximum size determinable at compile time,
> so the code has been reworked to use an ordinary array (without
> relying on the C99 variable-length array feature).  This always
> allows for a GPS_PATH_MAX-sized device_name, but the amount of
> space needed for that is relatively modest, and the behavior is
> more reproducible with a fixed allocation.
> 
> TESTED:
> Ran "scons build-all check" on OSX.  This code shouldn't be
> OS-dependent.
> ---
>  libgpsd_core.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/libgpsd_core.c b/libgpsd_core.c
> index 3fef4a8..eb4fddf 100644
> --- a/libgpsd_core.c
> +++ b/libgpsd_core.c
> @@ -204,26 +204,28 @@ const char *gpsd_prettydump(struct gps_device_t
> *session) session->lexer.outbuflen);
>  }
>  
> +/* Define the possible hook strings here so we can get the length */
> +#define HOOK_ACTIVATE "ACTIVATE"
> +#define HOOK_DEACTIVATE "DEACTIVATE"
> +
> +#define MAX(a,b) (((a) > (b)) ? (a) : (b))
> +#define HOOK_MAX MAX(sizeof(HOOK_ACTIVATE), sizeof(HOOK_DEACTIVATE))
> +#define HOOK_CMD_MAX (sizeof(DEVICEHOOKPATH)-1 + 1 + GPS_PATH_MAX-1
> + 1 \
> +                      + HOOK_MAX-1 + 1)
>  
>  static void gpsd_run_device_hook(struct gpsd_errout_t *errout,
>                                char *device_name, char *hook)
>  {
>      struct stat statbuf;
> +    char buf[HOOK_CMD_MAX];
> +
>      if (stat(DEVICEHOOKPATH, &statbuf) == -1)
>       gpsd_log(errout, LOG_PROG,
>                "no %s present, skipped running %s hook\n",
>                DEVICEHOOKPATH, hook);
>      else {
> -     /* use alloca(), which is not malloc(), here because
> -      * the pointer will never persist outside this small scope.
> -      */
> -     size_t bufsize = strlen(DEVICEHOOKPATH) + 1 +
> strlen(device_name) + 1 + strlen(hook) + 1;
> -     char *buf = alloca(bufsize);
> -     /* no need to check the return code of alloca()
> -         * by definition, if alloca() fails the program segfaults
> -         */
>       int status;
> -     (void)snprintf(buf, bufsize, "%s %s %s",
> +     (void)snprintf(buf, sizeof(buf), "%s %s %s",
>                      DEVICEHOOKPATH, device_name, hook);
>       gpsd_log(errout, LOG_INF, "running %s\n", buf);
>       status = system(buf);
> @@ -233,7 +235,6 @@ static void gpsd_run_device_hook(struct
> gpsd_errout_t *errout, gpsd_log(errout, LOG_INF,
>                    "%s returned %d\n", DEVICEHOOKPATH,
>                    WEXITSTATUS(status));
> -     /* buf automatically freed here as the stack pops */
>      }
>  }
>  
> @@ -356,7 +357,7 @@ void gpsd_deactivate(struct gps_device_t *session)
>      if (session->mode == O_OPTIMIZE)
>       gpsd_run_device_hook(&session->context->errout,
>                            session->gpsdata.dev.path,
> -                          "DEACTIVATE");
> +                          HOOK_DEACTIVATE);
>  #ifdef PPS_ENABLE
>      session->pps_thread.report_hook = NULL; /* tell any PPS-watcher
> thread to die */ #endif /* PPS_ENABLE */
> @@ -543,7 +544,7 @@ int gpsd_activate(struct gps_device_t *session,
> const int mode) {
>      if (mode == O_OPTIMIZE)
>       gpsd_run_device_hook(&session->context->errout,
> -                          session->gpsdata.dev.path, "ACTIVATE");
> +                          session->gpsdata.dev.path,
> HOOK_ACTIVATE); session->gpsdata.gps_fd = gpsd_open(session);
>      if (mode != O_CONTINUE)
>       session->mode = mode;




RGDS
GARY
---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97703
        address@hidden  Tel:+1 541 382 8588

pgp9LWIOJcCip.pgp
Description: OpenPGP digital signature