On Sat, Oct 13, 2012 at 09:55:22PM -0700, Junio C Hamano wrote:
> > When ~/.gitconfig is unreadable (EPERM), the messages are a symptom of
> > an older issue: the config file is being ignored. Shouldn't git error
> > out instead so the permissions can be fixed? E.g., if the sysadmin
> > has set "[branch] autoSetupRebase" to true in /etc/gitconfig and I
> > have set it to false in my own ~/.gitconfig, I'd rather see git error
> > out because ~/.gitconfig has become unreadable in a chmod gone wrong
> > than have a branch set up with the wrong settings and have to learn to
> > fix it up myself.
> >
> > In other words, how about something like this?
>
> I think that is a reasonable issue to address, but I wonder if we
> should be sharing more code between these. If the config side can
> be switched to unconditionally attempt to fopen and then deal with
> an error when it happens, we can get rid of access_or_{warn,die}
> and replace them with fopen_or_{warn,die} and use them from the two
> places (attr.c:read_attr_from_file() and the configuration stuff).
>
> I haven't looked to see if that a too intrusive refactoring to be
> worth it, though.
Handling the error at the fopen level would eliminate a minor race
condition (e.g., access() reports OK, then the file has its permissions
changed, then we fopen and get EPERM, but ignore it). So it would not
just be a refactoring, but would actually improve the code quality.
-Peff
--
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