Re: [PATCH] Make core.sharedRepository more generic

[Samuel Tardieu <sam@rfc1149.net>]

The use of named constants vs. literals seem inconsistent in your
patch, compare

| +               mode = (mode & ~0777) | shared_repository;

to

| +               mode |= (shared_repository & 0600) ? S_IXUSR : 0;
| +               mode |= (shared_repository & 0060) ? S_IXGRP : 0;
| +               mode |= (shared_repository & 0006) ? S_IXOTH : 0;

I first thought that you were using literals with "shared_repository"
and named constants with mode but the first line I quoted shows that
this is not the case.

Btw, aren't those last three lines better replaced by

  /* Copy read bits to execute bits */
  mode |= (shared_repository & 0444) >> 2;

I don't see where you deal with executable files.

Also, wouldn't it be more consistent to use a negative value to
--shared, that is a umask-compatible one, rather than a positive value
which needs to be tweaked for directories and executable files? You
would only have to "&" 0666 or 0777 with "~perms" to get the right
permissions.

--shared=0007 would be equivalent to PERM_GROUP, --shared=0027 to
group-readable-but-not-writable, and --shared=0002 to PERM_EVERYBODY.

  Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/