Re: [PATCH 1/5] Add platform-independent .git "symlink"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Feb 18, 2008 6:43 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Lars Hjemli <hjemli@xxxxxxxxx> writes:
>
> > This patch allows .git to be a regular textfile containing the path of
> > the real git directory (formatted like "gitdir: <path>\n"), which is
> > useful on platforms lacking support for real symlinks.
>
> I think a sane simplification is to allow the file to have
> any number of optional \r or \n at the end.

Agreed.

> > +     len = read_in_full(fd, buf, sizeof(buf));
> > +     close(fd);
> > +     if (len != st.st_size)
> > +             return NULL;
> > +     if (!len || buf[len - 1] != '\n')
> > +             return NULL;
> > +     buf[len - 1] = '\0';
> > +     if (prefixcmp(buf, "gitdir: "))
> > +             return NULL;
>
> But I am not sure about this part.  We found what claims to be
> the ".git" fake symlink but it is ill-formed.  Don't we want to
> diagnose the possible breakage for the user?

Yes, I think I got to eager in my 'gentleness'. It's probably better
to die() with an appropriate errormessage.

>
> > +/*
> > +     if (!is_git_directory(buf + 8))
> > +             return NULL;
> > +*/
>
> Likewise.

True, I'll uncomment and die().

Thanks for the review.

--
larsh
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux