Re: [PATCH] C version of git-count-objects

5 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] C version of git-count-objects

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:24

Nicolas Pitre [off-list ref] writes:
On Thu, 27 Apr 2006, Peter Hagervall wrote:
quoted
Answering the call Linus made[1], sort of, but for a completely
different program.

Anyway, it ought to be at least as portable as the shell script, and a
whole lot faster, however much that matters.
[...]
quoted
+	for (i = 0; i < 16; i++) {
+		subdir[0] = hex_digits[i];
+		for (j = 0; j < 16; j++) {
+			subdir[1] = hex_digits[j];
+			if (access(subdir, R_OK | X_OK))
+				continue;
+			chdir(subdir);
+			if (!(dp = opendir("."))) {
+				error("can't open subdir %s", subdir);
+				continue;
+			}
Looks like you're missing a chdir(".."); there.
Why would you even _need_ to chdir() anywhere, anyway?

Re: [PATCH] C version of git-count-objects

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:42:24

On Thu, 27 Apr 2006, Junio C Hamano wrote:
Nicolas Pitre [off-list ref] writes:
quoted
On Thu, 27 Apr 2006, Peter Hagervall wrote:
quoted
Answering the call Linus made[1], sort of, but for a completely
different program.

Anyway, it ought to be at least as portable as the shell script, and a
whole lot faster, however much that matters.
[...]
quoted
+	for (i = 0; i < 16; i++) {
+		subdir[0] = hex_digits[i];
+		for (j = 0; j < 16; j++) {
+			subdir[1] = hex_digits[j];
+			if (access(subdir, R_OK | X_OK))
+				continue;
+			chdir(subdir);
+			if (!(dp = opendir("."))) {
+				error("can't open subdir %s", subdir);
+				continue;
+			}
Looks like you're missing a chdir(".."); there.
Why would you even _need_ to chdir() anywhere, anyway?
To avoid appending the filename to the path before each lstat() I'd 
guess.


Nicolas

Re: [PATCH] C version of git-count-objects

From: Peter Hagervall <hidden>
Date: 2016-06-15 22:42:24

On Thu, Apr 27, 2006 at 03:39:14PM -0400, Nicolas Pitre wrote:
On Thu, 27 Apr 2006, Junio C Hamano wrote:
quoted
Nicolas Pitre [off-list ref] writes:
quoted
On Thu, 27 Apr 2006, Peter Hagervall wrote:
quoted
Answering the call Linus made[1], sort of, but for a completely
different program.

Anyway, it ought to be at least as portable as the shell script, and a
whole lot faster, however much that matters.
[...]
quoted
+	for (i = 0; i < 16; i++) {
+		subdir[0] = hex_digits[i];
+		for (j = 0; j < 16; j++) {
+			subdir[1] = hex_digits[j];
+			if (access(subdir, R_OK | X_OK))
+				continue;
+			chdir(subdir);
+			if (!(dp = opendir("."))) {
+				error("can't open subdir %s", subdir);
+				continue;
+			}
Looks like you're missing a chdir(".."); there.
Why would you even _need_ to chdir() anywhere, anyway?
To avoid appending the filename to the path before each lstat() I'd 
guess.
Yes, that's pretty much the reason.

	Peter

Re: [PATCH] C version of git-count-objects

From: Linus Torvalds <torvalds@osdl.org>
Date: 2016-06-15 22:42:24


On Thu, 27 Apr 2006, Peter Hagervall wrote:
quoted
To avoid appending the filename to the path before each lstat() I'd 
guess.
Yes, that's pretty much the reason.
It's a bad reason, though.

For one thing, it just doesn't work. You'll have to chdir() back, and you 
can't use ".." in case the user has set up some symlink thing. So you end 
up doing other really strange things.

You can do this much more efficiently with something like this:

	const char *obj = git_object_directory();
	int len = strlen(obj);
	char *dir = malloc(len + 300);

	memcpy(dir, obj, len);
	if (len && obj[len-1] != '/')
		dir[len++] = '/';
	dir[len+2] = 0;
	for (i = 0; i < 16; i++) {
		dir[len] = hexdigit[i];
		for (j = 0; j < 16; j+) {
			dir[len+1] = hexdigit[j];
			dir[len+2] = 0;
			DIR *d = opendir(dir);
			if (!d)
				continue;
			nr += count(d, dir, len+2);
			closedir(d);
		}
	}

where the "count()" function just ends up doing something like

	int count(DIR *d, const char *prefix, int len)
	{
		int nr = 0;
		struct dirent *de;

		prefix[len++] = '/';
		while ((de = readdir(d)) != NULL) {
			int fd;
			if (de->d_name[0] == '.')
				continue;
			strcpy(prefix + len, de->d_name);
			fd = open(prefix, O_RDONLY);
			.. check if it's ok, perhaps.. ?
			if (ok)
				nr++;
			close(fd);
		}
		return nr;
	}

and you're done. Efficient, and it's easy to add the endign to the 
pathname, because you're passing in a buffer that is big enough, and 
you're telling people where they should put their suffixes..

And no, the above has never been compiled or tested, and I wrote it with 
one eye closed, while drinking heavily and experimenting with some funky 
'shrooms. So caveat emptor.

		Linus

Re: [PATCH] C version of git-count-objects

From: Peter Hagervall <hidden>
Date: 2016-06-15 22:42:24

On Thu, Apr 27, 2006 at 01:07:27PM -0700, Linus Torvalds wrote:

On Thu, 27 Apr 2006, Peter Hagervall wrote:
quoted
quoted
To avoid appending the filename to the path before each lstat() I'd 
guess.
Yes, that's pretty much the reason.
It's a bad reason, though.

For one thing, it just doesn't work. You'll have to chdir() back, and you 
can't use ".." in case the user has set up some symlink thing. So you end 
up doing other really strange things.

You can do this much more efficiently with something like this:

	const char *obj = git_object_directory();
	int len = strlen(obj);
<snip>
				continue;
			strcpy(prefix + len, de->d_name);
			fd = open(prefix, O_RDONLY);
			.. check if it's ok, perhaps.. ?
			if (ok)
				nr++;
			close(fd);
		}
		return nr;
	}

and you're done. Efficient, and it's easy to add the endign to the 
pathname, because you're passing in a buffer that is big enough, and 
you're telling people where they should put their suffixes..
Thanks, I'll make a third stab at it tomorrow, if anyone is interested
that is?

	Peter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help