[apparmor] [PATCH 1/6] libapparmor: Use directory file descriptor in _aa_dirat_for_each()

[Tyler Hicks]

On 2015-04-01 17:51:15, John Johansen wrote:
> On 04/01/2015 02:37 PM, Seth Arnold wrote:
> > On Wed, Apr 01, 2015 at 12:49:30PM -0500, Tyler Hicks wrote:
> >>> I like the gist of this patch but I think the parameter shouldn't be named
> >>> dirfd; if we ever need the dirent.h header back again, this'll cause no
> >>> end of confusion. It'd be safer to just pick a less obvious name, dir_fd
> >>> for example.
> >>
> >> Thanks for the review!
> >>
> >> The "dirfd" naming convention is used in all of the *at(2) calls:
> >>
> >>  int faccessat(int dirfd, const char *pathname, int mode, int flags);
> >>  int fchmodat(int dirfd, const char *pathname, mode_t mode, int flags);
> >>  int fstatat(int dirfd, const char *pathname, struct stat *buf, int flags);
> >>  int openat(int dirfd, const char *pathname, int flags);
> > 
> > Interesting. I mean, dirfd is the first name that comes to mind for a
> > directory file descriptor, so it shouldn't be too surprising, but I am a
> > touch surprised collisions with dirfd(3) weren't considered first. Oh well.
> 
> Its a fn vs a variable, you can actually get away with them having the same
> name without a collision (well at least in C/C++ with a good compiler).

I tested this with gcc when Seth first brought it up. You can't have a
variable named dirfd and call dirfd(3) in the same scope.

$ cat dirfd.c
#include <sys/types.h>
#include <dirent.h>

int main(void)
{
	DIR *d;
	int dirfd;

	d = opendir(".");
	dirfd = dirfd(d);
	closedir(d);

	return 0;
}
$ gcc dirfd.c -o dirfd
dirfd.c: In function ‘main’:
dirfd.c:10:10: error: called object ‘dirfd’ is not a function or function pointer
  dirfd = dirfd(d);
          ^
dirfd.c:7:6: note: declared here
  int dirfd;
      ^

Moving them into different scopes is fine, though:

$ cat dirfd2.c
#include <sys/types.h>
#include <dirent.h>

int odir()
{
	return dirfd(opendir("."));
}

int main(void)
{
	int dirfd = odir(".");

	return 0;
}
$ gcc dirfd2.c -o dirfd2

Tyler

> 
> > 
> >>
> >> That said, I don't mind changing it. I think prefer "dfd" over "dir_fd".
> >> Does that sound good?
> > 
> > dfd is fine by me; I'm mostly interested in avoiding bikeshed discussions
> > but figured if I was going to complain about the original name I should in
> > all fairness have a suggestion. :)
> > 
> dirfd, dfd are both fine
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150401/ab9c00d6/attachment.pgp>