[PATCH] osutil: implementation for Win32

[Petr Kodl]

There are two more left - both PyList_Append and PyList_Sort can fail ...

and maybe

s/return ret ? ret : NULL; /return ret;

pk

On Wed, Sep 10, 2008 at 6:06 PM, Benoit Boissinot <bboissin at gmail.com>wrote:

> On Wed, Sep 10, 2008 at 10:05 PM, Petr Kodl <petrkodl at gmail.com> wrote:
> > As for the existing Unix code - it is actually buggy - it uses return
> values
> > from PyErr function as an indicator of an error, but that value is always
> > NULL - I think all of the Python PyErr functions do that  - so the final
> >
> > return err ? err : list
> >
> > is actually meaningless and what makes it work is the list being
> initialized
> > to NULL.
> >
> > There is still a potential bug if the code runs out of memory - than it
> can
> > really return partial list or list with partially initialized stat part
> and
> > the caller would never know about it.
>
> Possible patch below.
>
> Actually there are more problems. It seems the file descriptor isn't closed
> too.
>
> diff --git a/mercurial/osutil.c b/mercurial/osutil.c
> --- a/mercurial/osutil.c
> +++ b/mercurial/osutil.c
> @@ -158,7 +158,7 @@
>                Py_DECREF(val);
>        }
>
> -       return 0;
> +       return list;
>  }
>
>  static PyObject *statfiles(PyObject *list, PyObject *ctor_args, int keep,
> @@ -230,7 +230,7 @@
>                }
>        }
>
> -       return 0;
> +       return list;
>  }
>
>  static PyObject *listdir(PyObject *self, PyObject *args, PyObject *kwargs)
> @@ -239,7 +239,7 @@
>        DIR *dir = NULL;
>         PyObject *statobj = NULL;
>        PyObject *list = NULL;
> -       PyObject *err = NULL;
> +       PyObject *ret = NULL;
>        PyObject *ctor_args = NULL;
>         char *path;
>        char full_path[PATH_MAX + 10];
> @@ -262,7 +262,7 @@
>        dfd = -1;
>  #endif
>        if (!dir) {
> -               err = PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
> +               PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
>                goto bail;
>        }
>
> @@ -274,8 +274,8 @@
>        strncpy(full_path, path, PATH_MAX);
>        full_path[path_len] = '/';
>
> -       err = listfiles(list, dir, keep_stat, &need_stat);
> -       if (err)
> +       ret = listfiles(list, dir, keep_stat, &need_stat);
> +       if (!ret)
>                goto bail;
>
>        PyList_Sort(list);
> @@ -283,8 +283,8 @@
>        if (!keep_stat && !need_stat)
>                goto done;
>
> -       err = statfiles(list, ctor_args, keep_stat, full_path, path_len,
> dfd);
> -       if (!err)
> +       ret = statfiles(list, ctor_args, keep_stat, full_path, path_len,
> dfd);
> +       if (ret)
>                goto done;
>
>  bail:
> @@ -294,7 +294,7 @@
>        Py_XDECREF(ctor_args);
>        if (dir)
>                closedir(dir);
> -       return err ? err : list;
> +       return ret ? ret : NULL;
>  }
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://selenic.com/pipermail/mercurial-devel/attachments/20080910/7a130e43/attachment.htm