[PATCH 1 of 3] chg: let hgc_open support long path

Yuya Nishihara yuya at tcha.org
Sun Dec 25 05:29:12 EST 2016


On Fri, 23 Dec 2016 16:51:15 +0000, Jun Wu wrote:
> Excerpts from Jun Wu's message of 2016-12-23 16:46:48 +0000:
> > # HG changeset patch
> > # User Jun Wu <quark at fb.com>
> > # Date 1482511020 0
> > #      Fri Dec 23 16:37:00 2016 +0000
> > # Node ID 68566eb02a680d9a67fff8eb9ae3eab77872f886
> > # Parent  b0a0f7b9ed9007a3f1323f9417efad715489e918
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 68566eb02a68
> > chg: let hgc_open support long path
> > 
> > "sizeof(sun_path)" is too small. Use the chdir trick to support long socket
> > path, like "mercurial.util.bindunixsocket".
> > 
> > It's useful for cases where TMPDIR is long. Modern OS X rewrites TMPDIR to a
> > long value. And we probably want to use XDG_RUNTIME_DIR [2] for Linux.
> > 
> > The approach is a bit different from the previous plan, where we will have
> > hgc_openat and pass cmdserveropts.sockdirfd to it. That's because the
> > current change is easier: chg has to pass a full path to "hg" as the
> > "--address" parameter. There is no "--address-basename" or "--address-dirfd"
> > flags. The next patch will remove "sockdirfd".
> > 
> > Note: It'd be nice if we can use a native "connectat" implementation.
> > However, that's not available everywhere. Some platform (namely FreeBSD)
> > does support it, but the implementation has bugs so it cannot be used [2].
> > 
> > [1]: https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html 
> > [2]: https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-April/082892.html 
> > 
> > diff --git a/contrib/chg/hgclient.c b/contrib/chg/hgclient.c
> > --- a/contrib/chg/hgclient.c
> > +++ b/contrib/chg/hgclient.c
> > @@ -426,8 +426,41 @@ hgclient_t *hgc_open(const char *socknam
> >      struct sockaddr_un addr;
> >      addr.sun_family = AF_UNIX;
> > -    strncpy(addr.sun_path, sockname, sizeof(addr.sun_path));
> > +
> > +    /* use chdir to workaround small sizeof(sun_path) */
> > +    int bakfd = -1;
> > +    const char *basename = sockname;
> > +    {
> > +        const char *split = strrchr(sockname, '/');
> > +        if (split) {
> 
> I just realized that this line should be changed to:
> 
>            if (split && split != sockname)
> 
> to support socket paths like "/foo".

Fixed in flight and queued the series, thanks.

> > +    /* real connect */
> >      int r = connect(fd, (struct sockaddr *)&addr, sizeof(addr));

Perhaps we should back up (errno == ENOENT || errno == ECONNREFUSED) here
before doing another syscall, but that isn't the issue introduced by this patch.
(and the original code doesn't care a close() failure.)

> > +    if (bakfd != -1) {
> > +        fchdirx(bakfd);
> > +        close(bakfd);
> > +    }
> > +
> >      if (r < 0) {
> >          close(fd);
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list