Re: svn_dirent_t.size API inconsistency

On 21.12.2017 15:14, Stefan Fuhrmann wrote:
> I think I found an API documentation bug.
> svn_types.h specifies for svn_dirent_t.size:
>
>   /** length of file text, or 0 for directories */
>   svn_filesize_t size;
>
> However, (almost?) all implementations set it to
> SVN_INVALID_FILESIZE for directories. This is also
> what we do for svn_io_dirent_t.size.
>
> I discovered this as r1818578 broke our Perl tests.
> The same test makes it clear that no API user could
> have successfully used the API as documented.
>
> Therefore, I suggest to change the API doc to match
> the implementation, check the code for consistency
> and add an entry to our errata list.
>
> -- Stefan^2.
>
Below the necessary patch w/o erratum.

-- Stefan^2.

[[[
Fix inconsistent handling of svn_dirent_t.size for directories.

Most code uses SVN_INVALID_FILESIZE for them, so change the docs to match
that fact and update all users to consistently follow the new docstring.

* subversion/include/svn_types.h
 Â  (svn_dirent_t): Change documentation for SIZE.

* subversion/libsvn_ra_local/ra_plugin.c
 Â  (svn_ra_local__get_dir): Return the new default for directories.

* subversion/libsvn_ra_serf/list.c
 Â  (item_closed): Be sure to default to SVN_INVALID_FILESIZE.

* subversion/libsvn_repos/list.c
 Â  (fill_dirent): Set SIZE consistently with other RA layers.

--This line, and those below, will be ignored--

Index: subversion/include/svn_types.h
===================================================================
--- subversion/include/svn_types.h    (revision 1818804)
+++ subversion/include/svn_types.h    (working copy)
@@ -652,7 +652,7 @@ typedef struct svn_dirent_t
 Â Â  /** node kind */
 Â Â  svn_node_kind_t kind;

-  /** length of file text, or 0 for directories */
+  /** length of file text, otherwise SVN_INVALID_FILESIZE */
 Â Â  svn_filesize_t size;

 Â Â  /** does the node have props? */
Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c    (revision 1818804)
+++ subversion/libsvn_ra_local/ra_plugin.c    (working copy)
@@ -1387,7 +1387,7 @@ svn_ra_local__get_dir(svn_ra_session_t *
 Â Â Â Â Â Â Â Â Â Â Â Â  {
 Â Â Â Â Â Â Â Â Â Â Â Â Â Â  /* size  */
 Â Â Â Â Â Â Â Â Â Â Â Â Â Â  if (fs_entry->kind == svn_node_dir)
-                entry->size = 0;
+                entry->size = SVN_INVALID_FILESIZE;
 Â Â Â Â Â Â Â Â Â Â Â Â Â Â  else
 Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  SVN_ERR(svn_fs_file_length(&(entry->size), root,
 Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  fullpath, iterpool));
Index: subversion/libsvn_ra_serf/list.c
===================================================================
--- subversion/libsvn_ra_serf/list.c    (revision 1818932)
+++ subversion/libsvn_ra_serf/list.c    (working copy)
@@ -139,6 +139,8 @@ item_closed(svn_ra_serf__xml_estate_t *x

 Â Â Â Â Â Â  if (size)
 Â Â Â Â Â Â Â Â  SVN_ERR(svn_cstring_atoi64(&dirent.size, size));
+      else
+        dirent.size = SVN_INVALID_FILESIZE;

 Â Â Â Â Â Â  if (crev)
 Â Â Â Â Â Â Â Â  SVN_ERR(svn_revnum_parse(&dirent.created_rev, crev, NULL));
Index: subversion/libsvn_repos/list.c
===================================================================
--- subversion/libsvn_repos/list.c    (revision 1818804)
+++ subversion/libsvn_repos/list.c    (working copy)
@@ -51,7 +51,7 @@ fill_dirent(svn_dirent_t *dirent,
 Â Â  if (dirent->kind == svn_node_file)
 Â Â Â Â  SVN_ERR(svn_fs_file_length(&(dirent->size), root, path,
scratch_pool));
 Â Â  else
-    dirent->size = 0;
+    dirent->size = SVN_INVALID_FILESIZE;

 Â Â  SVN_ERR(svn_fs_node_has_props(&dirent->has_props, root, path,
 Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  scratch_pool));
]]]
Received on 2017-12-21 16:01:38 CET