[PATCH 2 of 2 V3] osutil: implement setprocname to set process title for some platforms

[Jun Wu]

Excerpts from Yuya Nishihara's message of 2016-11-14 21:16:48 +0900:
> On Sun, 13 Nov 2016 15:25:46 +0000, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark at fb.com>
> > # Date 1478898677 0
> > #      Fri Nov 11 21:11:17 2016 +0000
> > # Node ID c4af4fec293bed4e5105442137a24c34d58e740f
> > # Parent  98761d64eaaf67f3bdb99f3f80a57910e2624b78
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r c4af4fec293b
> > osutil: implement setprocname to set process title for some platforms
> 
> > +#ifndef SETPROCNAME_USE_NONE
> > +static PyObject *setprocname(PyObject *self, PyObject *args)
> > +{
> > +    const char *name = NULL;
> > +    if (!PyArg_ParseTuple(args, "s", &name))
> > +        return NULL;
> > +
> > +#if defined(SETPROCNAME_USE_SETPROCTITLE)
> > +    setproctitle("%s", name);
> > +#elif defined(SETPROCNAME_USE_ARGVREWRITE)
> > +    {
> > +        static char *argvstart = NULL;
> > +        static size_t argvsize = 0;
> > +        if (argvstart == NULL) {
> > +            int argc = 0, i;
> > +            char **argv = NULL;
> > +            extern void Py_GetArgcArgv(int *argc, char ***argv);
> > +            Py_GetArgcArgv(&argc, &argv);
> > +
> > +            /* Check the memory we can use. Typically, argv[i] and
> > +             * argv[i + 1] are continuous. */
> > +            for (argvstart = argv[0], i = 0; i < argc; ++i) {
> > +                if (argv[i] > argvstart + argvsize)
> > +                    break; /* not continuous */
> > +                size_t len = strlen(argv[i]);
> > +                argvsize = argv[i] - argv[0] + len + 1;
> > +            }
> 
> Segfaults if argv[i] < argv[0].
> 
>   % gdb python
>   (gdb) run -c 'from mercurial import osutil; osutil.setprocname("foo")'
>   (gdb) p argc
>   $25 = 3
>   (gdb) p argv[0]
>   $28 = 0x7fffffffe307 "/usr/bin/python"
>   (gdb) p argv[1]
>   $29 = 0x7fffffffe317 "-c"
>   (gdb) p argv[2]
>   $30 = 0x555555763dd8 "-c"

Good find! I'm a bit curious about what gdb does exactly here.

> I think the original "argv[i] == argvstart + argvsize" was simple enough. We
> can't calculate the exact capacity of underlying buffer anyway once it's
> clobbered (and zero-filled.)

The V3 change is intended to handle this case (which wasn't handled in V2):
  
  argv[] = {"foobar", "bar", NULL}
  "foobar"
   |  |
   |  +--- argv[1]
   +------ argv[0]

I think the most correct fix would probably be checking the left boundary as
well. Maybe a "argvend" is better than "argvsize".

> > +        if (argvstart && argvsize > 1) {
> > +            int n = snprintf(argvstart, argvsize, "%s", name);
> > +            if (n >= 0 && (size_t)n < argvsize)
> > +                memset(argvstart + n, 0, argvsize - n);
> > +        }
> > +    }