Re: [Nano-devel] Patch for bug #44950

[Benno Schulenberg]

On Sat, Jan 23, 2016, at 15:59, Rishabh Dave wrote:
> Ehmm... I am confused about the 'not-accessible.patch'. Code in repository
> is different. Was I supposed to work with it patched to source from repo?

Yes, you should have applied the patch to the current code
in the repo.  That's why it's called a patch.  When it had
already been applied, I would have just said to look at
revision rnnnn.

> I have created a new function for isolating code and faults in it (all of
> this under 'check-permissions.patch'). Should we keep it that way if it
> works?

No -- it duplicates more than half of the other function.

> We could merge it with 'has_valid_path()' as it is pretty similar
> and task performed is also much coherent.

Of course.  But your patch does not do what I want.  If I run
'src/nano /root/.nano/yuhu', with your patch nano says:

  [ Directory '/root/.nano' does not exist ]

Which is false, because I know that /root/.nano exists.
With my patch it says:

  [ Path '/root/.nano': Permission denied ]

Which is correct.

Also, your patch goes back up through all the components
of the path?  Why?

> I have doubt for following pair
> of line under my own patch -
> 
> +    if (strcmp(parentdir, ".") == 0)
> +        parentdir = mallocstrcpy(NULL, filename);
> +    free(parentdir);
> +    return validity;
> 
> 'if-statement' is used to avoid 'free(): invalid pointer' error. It is not
> exactly good programming practice, is it?

Ugly.  Better turn the if around:
  if (strcmp() != 0)
      free(parentdir);

> Besides, source file 'proto.h' has return type of 'has_valid_path()' as
> void. I changed it to bool as it is bool under file 'files.c'.

Yes, that was an oversight; I should have removed it from
proto.h -- I don't put functions in there that aren't used
outside of the file they are in.

Oh, yes, one more thing: you create your patches with 'svn diff',
but please do that in the main dir of the repo, not in the src/ subdir.

Oh, and yes, it's about time to start a new thread.  :)

Benno

-- 
http://www.fastmail.com - The way an email service should be