On Tue, Oct 20, 2020 at 02:57:22PM -0000, Christian Weisgerber wrote:
> On 2020-10-20, Stefan Sperling <stsp@stsp.name> wrote:
> 
> > This patch should make got and tog work with non-const dirname(3).
> 
> > Switch path comparisons from strcmp() to got_path_cmp() in order to
> > treat paths such as "foo//bar" and "foo/bar" as equal.
> 
> It also treats "/foo/bar" and "foo/bar" as equal.  Could that be a
> problem?

Good point. For the changes made in this diff, all paths involved should
already be absolute. So I expect it will be fine.

> > --- lib/worktree.c
> > +++ lib/worktree.c
> > @@ -2018,16 +2047,22 @@ remove_ondisk_file(const char *root_path, const char *
> >  		if (errno != ENOENT)
> >  			err = got_error_from_errno2("unlink", ondisk_path);
> >  	} else {
> > -		char *parent = dirname(ondisk_path);
> > -		while (parent && strcmp(parent, root_path) != 0) {
> > -			if (rmdir(parent) == -1) {
> > +		size_t root_len = strlen(root_path);
> > +		do {
> > +			char *parent;
> > +			err = got_path_dirname(&parent, ondisk_path);
> > +			if (err)
> > +				return err;
> > +			free(ondisk_path);
> 
> The free() needs to move up, otherwise there's a leak on error.
> 
> > @@ -3877,25 +3914,22 @@ schedule_for_deletion(void *arg, unsigned char status,
> >  			goto done;
> >  		}
> >  
> > -		parent = dirname(ondisk_path);
> > -
> > -		if (parent == NULL) {
> > -			err = got_error_from_errno2("dirname", ondisk_path);
> > -			goto done;
> > -		}
> > -		while (parent && strcmp(parent, a->worktree->root_path) != 0) {
> > -			if (rmdir(parent) == -1) {
> > +		root_len = strlen(a->worktree->root_path);
> > +		do {
> > +			char *parent;
> > +			err = got_path_dirname(&parent, ondisk_path);
> > +			if (err)
> > +				return err;
> > +			free(ondisk_path);
> 
> Same here.

Thanks. I will plug these leaks as follows:

diff 8340e30f622b33fbf0e18dc2b494c6d65e24aea3 /home/stsp/src/got
blob - a46191b6be513b0aa32c2a166157fe86ee15aefe
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -2052,7 +2052,7 @@ remove_ondisk_file(const char *root_path, const char *
 			char *parent;
 			err = got_path_dirname(&parent, ondisk_path);
 			if (err)
-				return err;
+				break;
 			free(ondisk_path);
 			ondisk_path = parent;
 			if (rmdir(ondisk_path) == -1) {
@@ -3919,7 +3919,7 @@ schedule_for_deletion(void *arg, unsigned char status,
 			char *parent;
 			err = got_path_dirname(&parent, ondisk_path);
 			if (err)
-				return err;
+				goto done;
 			free(ondisk_path);
 			ondisk_path = parent;
 			if (rmdir(ondisk_path) == -1) {