Re: [PATCH] with log Re: svn_auth_cred_simple_t crash

[Martin Hauner <ha...@web.de>]

... and of course, i forgot the log message to :(

-- 
Martin

Subcommander, http://subcommander.tigris.org
a x-platform Win32/Unix svn gui (qt) client & a text diff/merge tool.

Re: [PATCH] with log Re: svn_auth_cred_simple_t crash

[Max Bowsher <ma...@ukf.net>]

Martin Hauner wrote:
> Greg Hudson wrote:
>> We're unlikely to accept this patch.  We recently decided that if we
>> document "the pointer must not be NULL" in one place, we are compelled
>> to document that everywhere (as people who read the restriction in one
>> place might begin to assume that the restriction only applies when
>> explicitly mentioned), and that's too wordy.
>
> I general i agree, but in this case i would prefer a note.

What we need, is a more obvious place to document things which are common 
across our whole API.

The "NULL not allowed unless explicitly stated" rule is common knowledge 
amongst those who actually hack on Subversion a lot, but I think the only 
place it is documented right now is in HACKING, which isn't exactly API 
docs.

On the other hand, passing NULL to something when you haven't been told it 
is explicitly allowed is kind of tempting fate.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] with log Re: svn_auth_cred_simple_t crash

[Martin Hauner <ha...@web.de>]

Greg Hudson wrote:
> We're unlikely to accept this patch.  We recently decided that if we
> document "the pointer must not be NULL" in one place, we are compelled
> to document that everywhere (as people who read the restriction in one
> place might begin to assume that the restriction only applies when
> explicitly mentioned), and that's too wordy.

I general i agree, but in this case i would prefer a note.

> I don't know where you got the idea that passwords are "kind of
> optional".  We have a separate auth provider for just the username.

Because I can't know in advance if the user will enter a password or not.


Anyway, I'll stop here..

I have already fixed the problem on my side and maybe other users of
svn_auth_cred_simple_t are smarter than me and and don't fall over
this one. ;-)

-- 
Martin

Subcommander, http://subcommander.tigris.org
a x-platform Win32/Unix svn gui (qt) client & a text diff/merge tool.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] with log Re: svn_auth_cred_simple_t crash

[Greg Hudson <gh...@MIT.EDU>]

We're unlikely to accept this patch.  We recently decided that if we
document "the pointer must not be NULL" in one place, we are compelled
to document that everywhere (as people who read the restriction in one
place might begin to assume that the restriction only applies when
explicitly mentioned), and that's too wordy.

I don't know where you got the idea that passwords are "kind of
optional".  We have a separate auth provider for just the username.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] with log Re: svn_auth_cred_simple_t crash

[Julian Foad <ju...@btopenworld.com>]

I agree that we should not explicity document that pointers need to be 
non-null, except as a clarification (probably parenthesised to indicate that 
this is a clarification).

Martin Hauner wrote:
> 
> Index: subversion/include/svn_auth.h
> ===================================================================
> --- subversion/include/svn_auth.h	(revision 13396)
> +++ subversion/include/svn_auth.h	(working copy)
> @@ -170,9 +170,11 @@
>  /** @c SVN_AUTH_CRED_SIMPLE credentials. */
>  typedef struct svn_auth_cred_simple_t
>  {
> -  /** Username */
> +  /** Username. Expected to be non zero. Use the empty string (ie. "") for
> +   * and empty or unspecified username.
> +   */

If the user name is indeed optional, the comment should say so, and an 
appropriate comment would be:

   /** User name. May be the empty string to indicate an unknown user. */

or

   /** User name. May be the empty string (not NULL) to indicate an unknown 
user. */


>    const char *username;
> -  /** Password */
> +  /** Password. Expected to be non zero. See above. */
>    const char *password;

The phrase "See above" is not specific enough to be used in a documentation 
comment.  (There is lots of text above that point, and any or all of it may be 
moved or changed in future.)

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org