[PATCH 2 of 4 v2] mpatch: shuffle stuff around so it's easier to make a cffi version of mpatch

Yuya Nishihara yuya at tcha.org
Tue Jun 28 08:37:46 EDT 2016


On Mon, 27 Jun 2016 16:37:03 +0200, Maciej Fijalkowski wrote:
> On Mon, Jun 27, 2016 at 3:54 PM, Yuya Nishihara <yuya at tcha.org> wrote:
> > On Thu, 23 Jun 2016 08:57:09 +0200, Maciej Fijalkowski wrote:  
> >> # HG changeset patch
> >> # User Maciej Fijalkowski <fijall at gmail.com>
> >> # Date 1465303444 -7200
> >> #      Tue Jun 07 14:44:04 2016 +0200
> >> # Node ID f5a166fffae1ade1e5b7bcad759a3899813bd91f
> >> # Parent  d62df3ee14e3b2cf658c247233bda60def219e0f
> >> mpatch: shuffle stuff around so it's easier to make a cffi version of mpatch  
> >  
> >> +static struct flist *lalloc(ssize_t size)  
> >
> > IIRC, ssize_t is available only on posix environment. We'll need typedef for
> > Windows.
> >  
> >> -static struct flist *decode(const char *bin, Py_ssize_t len)
> >> +int decode(const char *bin, ssize_t len, struct flist** res)
> >>  {
> >>       struct flist *l;
> >>       struct frag *lt;
> >> @@ -208,7 +195,7 @@
> >>       /* assume worst case size, we won't have many of these lists */
> >>       l = lalloc(len / 12 + 1);
> >>       if (!l)
> >> -             return NULL;
> >> +             return MPATCH_ERR_NO_MEM;  
> >
> > It would greatly help reviewing patches if error handling changes were
> > made in a separate patch.  
> 
> I would vastly prefer not to do this - removing Python.h from includes
> is the point of this patch (otherwise you end up in a strange state
> where you shuffle stuff around, btu it's actually useless)

Mixing code moves and design changes makes the patch hard to review. That's
my point. I had to make sure no error handling lost from the original C
extension, which wasn't easy. But that might be no problem for someone else.


More information about the Mercurial-devel mailing list