Re: svn commit: r995383 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

2010-09-09 Thread Daniel Shahaf
s...@apache.org wrote on Thu, Sep 09, 2010 at 11:31:13 -:
 +  return svn_error_return(
 +   svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
 +_(Malformed representation header)));

svn_error_return() isn't necessary here, since svn_error_create()
has always recorded the file-line coordinates.

That's unlike, say, svn_error_compose_create(), which doesn't record the
file-line coordinates; thus,

err1 = foo();
err2 = bar();
return svn_error_return(svn_error_compose_create(err1, err2));

isn't redundant.


Re: svn commit: r995383 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

2010-09-09 Thread Stefan Sperling
On Thu, Sep 09, 2010 at 03:20:17PM +0300, Daniel Shahaf wrote:
 s...@apache.org wrote on Thu, Sep 09, 2010 at 11:31:13 -:
  +  return svn_error_return(
  +   svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
  +_(Malformed representation header)));
 
 svn_error_return() isn't necessary here, since svn_error_create()
 has always recorded the file-line coordinates.
 
 That's unlike, say, svn_error_compose_create(), which doesn't record the
 file-line coordinates; thus,
 
 err1 = foo();
 err2 = bar();
 return svn_error_return(svn_error_compose_create(err1, err2));
 
 isn't redundant.

Ah. Is the rule documented somewhere?


Re: svn commit: r995383 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

2010-09-09 Thread Daniel Shahaf
Stefan Sperling wrote on Thu, Sep 09, 2010 at 14:27:23 +0200:
 On Thu, Sep 09, 2010 at 03:20:17PM +0300, Daniel Shahaf wrote:
  s...@apache.org wrote on Thu, Sep 09, 2010 at 11:31:13 -:
   +  return svn_error_return(
   +   svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
   +_(Malformed representation header)));
  
  svn_error_return() isn't necessary here, since svn_error_create()
  has always recorded the file-line coordinates.
  
  That's unlike, say, svn_error_compose_create(), which doesn't record the
  file-line coordinates; thus,
  
  err1 = foo();
  err2 = bar();
  return svn_error_return(svn_error_compose_create(err1, err2));
  
  isn't redundant.
 
 Ah. Is the rule documented somewhere?

Not that I know.  (I (past tense)read svn_error.h.)

It might be good to make the rule 'return svn_error_*();' never needs
svn_error_return() wrapper and fix the svn_error_* functions/macros
that don't already record the file/line.


Re: svn commit: r995383 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

2010-09-09 Thread Stefan Sperling
On Thu, Sep 09, 2010 at 03:39:04PM +0300, Daniel Shahaf wrote:
 It might be good to make the rule 'return svn_error_*();' never needs
 svn_error_return() wrapper and fix the svn_error_* functions/macros
 that don't already record the file/line.

+1