> -----Original Message-----
> From: Julian Foad [mailto:[email protected]]
> Sent: maandag 21 december 2009 16:05
> To: Stefan Sperling
> Cc: Philip Martin; Mark Phippard; [email protected]
> Subject: Re: svn commit: r884250 -
> /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
>
> Stefan Sperling wrote:
> > - assert(strlen(txn_id) < sizeof(txn->txn_id));
> > - strncpy(txn->txn_id, txn_id, sizeof(txn->txn_id) - 1);
> > - txn->txn_id[sizeof(txn->txn_id) - 1] = '\0';
> > + end = apr_cpystrn(txn->txn_id, txn_id, sizeof(txn->txn_id));
> > + if (end < txn->txn_id + strlen(txn_id))
> > + return svn_error_createf(SVN_ERR_FS_TXN_NAME_TOO_LONG,
> NULL,
> > + _("Transaction name '%s' was truncated"),
> > txn_id);
>
> The "was truncated" error message is not helpful. (That sounds like the
> server is saying, "I have renamed one of your transactions".) Any reason
> not to use SVN_ERR_ASSERT?
>
> The logic looks correct.
Assertions are not nice to library users. (Not everybody uses 'svn' as his/her
subversion client). An assertion/abort in release code will most likely make my
users loose at least some of their work.
I don't know if this case is user and or network triggerable, but littering our
code with assertions makes our code slow and easier to crash, while the error
return method was designed to be more graceful.
I guess this code is server side. For this case errors are transferred to the
client, while a server assertion will only be visible as 'connection closed
unexpectedly' or something similar.
Maybe assertions and aborts are easier for Subversion developers, but they are
not for our users.
Bert
