You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2013/10/24 17:00:13 UTC

svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Author: philip
Date: Thu Oct 24 15:00:12 2013
New Revision: 1535399

URL: http://svn.apache.org/r1535399
Log:
Cast NULL to (char *) in vararg calls to svn_dirent_join_many
and svn_path_join_many.

* subversion/libsvn_fs_fs/lock.c
  (digest_path_from_digest, digest_path_from_path): here

* subversion/libsvn_fs_fs/transaction.c
  (create_txn_dir, create_txn_dir_pre_1_5): here
 
* subversion/libsvn_fs_fs/util.c
  (svn_fs_fs__path_rev_packed, svn_fs_fs__path_rev_shard,
   svn_fs_fs__path_rev, svn_fs_fs__path_revprops_shard,
   svn_fs_fs__path_revprops_pack_shard, svn_fs_fs__path_revprops,
   svn_fs_fs__path_txn_dir, svn_fs_fs__path_txn_proto_rev,
   svn_fs_fs__path_txn_proto_rev_lock, svn_fs_fs__path_node_origin): here

* subversion/libsvn_fs_x/lock.c
  (digest_path_from_digest, digest_path_from_path): here
 
* subversion/libsvn_fs_x/transaction.c
  (create_txn_dir): here

* subversion/libsvn_fs_x/util.c
  (svn_fs_x__path_rev_packed, svn_fs_x__path_rev_shard,
   svn_fs_x__path_revprops_shard,
   svn_fs_x__path_revprops_pack_shard,
   svn_fs_x__path_txn_dir, svn_fs_x__path_txn_proto_rev,
   svn_fs_x__path_txn_proto_rev_lock, svn_fs_x__path_node_origin): here
 
* subversion/libsvn_subr/config_file.c
  (svn_config__sys_config_path, svn_config_get_user_config_path): here
 
* subversion/libsvn_wc/adm_files.c
  (svn_wc__adm_child): here
 
* subversion/libsvn_wc/upgrade.c
  (build_lockfile_path, bump_to_29): here
 
* subversion/libsvn_wc/wc_db.c
  (svn_wc__db_temp_wcroot_tempdir): here
 
* subversion/libsvn_wc/wc_db_pristine.c
  (get_pristine_fname): here
 
* subversion/mod_authz_svn/mod_authz_svn.c
  (get_access_conf): here

* subversion/tests/cmdline/entries-dump.c
  (entries_dump): here

* subversion/tests/libsvn_client/client-test.c
  (test_patch): here
 
* subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
  (pack_filesystem, pack_even_filesystem, recover_fully_packed): here

* subversion/tests/libsvn_fs_x/fs-x-pack-test.c
  (pack_filesystem, pack_even_filesystem, recover_fully_packed): here

* subversion/tests/libsvn_subr/dirent_uri-test.c
  (test_dirent_join): here

* subversion/tests/libsvn_subr/path-test.c
  (test_path_join): here
 
* subversion/tests/libsvn_wc/entries-compat.c
  (test_entries_alloc, test_stubs): here

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/lock.c
    subversion/trunk/subversion/libsvn_fs_fs/transaction.c
    subversion/trunk/subversion/libsvn_fs_fs/util.c
    subversion/trunk/subversion/libsvn_fs_x/lock.c
    subversion/trunk/subversion/libsvn_fs_x/transaction.c
    subversion/trunk/subversion/libsvn_fs_x/util.c
    subversion/trunk/subversion/libsvn_subr/config_file.c
    subversion/trunk/subversion/libsvn_wc/adm_files.c
    subversion/trunk/subversion/libsvn_wc/upgrade.c
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c
    subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c
    subversion/trunk/subversion/tests/cmdline/entries-dump.c
    subversion/trunk/subversion/tests/libsvn_client/client-test.c
    subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
    subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c
    subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
    subversion/trunk/subversion/tests/libsvn_subr/path-test.c
    subversion/trunk/subversion/tests/libsvn_wc/entries-compat.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/lock.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/lock.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/lock.c Thu Oct 24 15:00:12 2013
@@ -133,7 +133,7 @@ digest_path_from_digest(const char *fs_p
 {
   return svn_dirent_join_many(pool, fs_path, PATH_LOCKS_DIR,
                               apr_pstrmemdup(pool, digest, DIGEST_SUBDIR_LEN),
-                              digest, NULL);
+                              digest, (char *)NULL);
 }
 
 
@@ -151,7 +151,7 @@ digest_path_from_path(const char **diges
   *digest_path = svn_dirent_join_many(pool, fs_path, PATH_LOCKS_DIR,
                                       apr_pstrmemdup(pool, digest,
                                                      DIGEST_SUBDIR_LEN),
-                                      digest, NULL);
+                                      digest, (char *)NULL);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Thu Oct 24 15:00:12 2013
@@ -1000,7 +1000,7 @@ create_txn_dir(const char **id_p,
                                  PATH_TXNS_DIR,
                                  apr_pstrcat(pool, *id_p, PATH_EXT_TXN,
                                              (char *)NULL),
-                                 NULL);
+                                 (char *)NULL);
 
   return svn_io_dir_make(txn_dir, APR_OS_DEFAULT, pool);
 }
@@ -1024,7 +1024,7 @@ create_txn_dir_pre_1_5(const char **id_p
 
   /* Try to create directories named "<txndir>/<rev>-<uniqueifier>.txn". */
   prefix = svn_dirent_join_many(pool, fs->path, PATH_TXNS_DIR,
-                                apr_psprintf(pool, "%ld", rev), NULL);
+                                apr_psprintf(pool, "%ld", rev), (char *)NULL);
 
   subpool = svn_pool_create(pool);
   for (i = 1; i <= 99999; i++)

Modified: subversion/trunk/subversion/libsvn_fs_fs/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/util.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/util.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/util.c Thu Oct 24 15:00:12 2013
@@ -98,7 +98,7 @@ svn_fs_fs__path_rev_packed(svn_fs_t *fs,
                               apr_psprintf(pool,
                                            "%ld" PATH_EXT_PACKED_SHARD,
                                            rev / ffd->max_files_per_dir),
-                              kind, NULL);
+                              kind, (char *)NULL);
 }
 
 const char *
@@ -110,7 +110,7 @@ svn_fs_fs__path_rev_shard(svn_fs_t *fs, 
   return svn_dirent_join_many(pool, fs->path, PATH_REVS_DIR,
                               apr_psprintf(pool, "%ld",
                                                  rev / ffd->max_files_per_dir),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -128,7 +128,7 @@ svn_fs_fs__path_rev(svn_fs_t *fs, svn_re
     }
 
   return svn_dirent_join_many(pool, fs->path, PATH_REVS_DIR,
-                              apr_psprintf(pool, "%ld", rev), NULL);
+                              apr_psprintf(pool, "%ld", rev), (char *)NULL);
 }
 
 const char *
@@ -155,7 +155,7 @@ svn_fs_fs__path_revprops_shard(svn_fs_t 
   return svn_dirent_join_many(pool, fs->path, PATH_REVPROPS_DIR,
                               apr_psprintf(pool, "%ld",
                                            rev / ffd->max_files_per_dir),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -169,7 +169,7 @@ svn_fs_fs__path_revprops_pack_shard(svn_
   return svn_dirent_join_many(pool, fs->path, PATH_REVPROPS_DIR,
                               apr_psprintf(pool, "%ld" PATH_EXT_PACKED_SHARD,
                                            rev / ffd->max_files_per_dir),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -187,7 +187,7 @@ svn_fs_fs__path_revprops(svn_fs_t *fs,
     }
 
   return svn_dirent_join_many(pool, fs->path, PATH_REVPROPS_DIR,
-                              apr_psprintf(pool, "%ld", rev), NULL);
+                              apr_psprintf(pool, "%ld", rev), (char *)NULL);
 }
 
 /* Return TO_ADD appended to the C string representation of TXN_ID.
@@ -211,7 +211,7 @@ svn_fs_fs__path_txn_dir(svn_fs_t *fs,
   return svn_dirent_join_many(pool, fs->path, PATH_TXNS_DIR,
                               combine_txn_id_string(txn_id, PATH_EXT_TXN,
                                                     pool),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -224,7 +224,7 @@ svn_fs_fs__path_txn_proto_rev(svn_fs_t *
     return svn_dirent_join_many(pool, fs->path, PATH_TXN_PROTOS_DIR,
                                 combine_txn_id_string(txn_id, PATH_EXT_REV,
                                                       pool),
-                                NULL);
+                                (char *)NULL);
   else
     return svn_dirent_join(svn_fs_fs__path_txn_dir(fs, txn_id, pool),
                            PATH_REV, pool);
@@ -241,7 +241,7 @@ svn_fs_fs__path_txn_proto_rev_lock(svn_f
                                 combine_txn_id_string(txn_id,
                                                       PATH_EXT_REV_LOCK,
                                                       pool),
-                                NULL);
+                                (char *)NULL);
   else
     return svn_dirent_join(svn_fs_fs__path_txn_dir(fs, txn_id, pool),
                            PATH_REV_LOCK, pool);
@@ -292,7 +292,7 @@ svn_fs_fs__path_node_origin(svn_fs_t *fs
     buffer[len - 1] = '\0';
 
   return svn_dirent_join_many(pool, fs->path, PATH_NODE_ORIGINS_DIR,
-                              buffer, NULL);
+                              buffer, (char *)NULL);
 }
 
 const char *

Modified: subversion/trunk/subversion/libsvn_fs_x/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/lock.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/lock.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/lock.c Thu Oct 24 15:00:12 2013
@@ -134,7 +134,7 @@ digest_path_from_digest(const char *fs_p
 {
   return svn_dirent_join_many(pool, fs_path, PATH_LOCKS_DIR,
                               apr_pstrmemdup(pool, digest, DIGEST_SUBDIR_LEN),
-                              digest, NULL);
+                              digest, (char *)NULL);
 }
 
 
@@ -152,7 +152,7 @@ digest_path_from_path(const char **diges
   *digest_path = svn_dirent_join_many(pool, fs_path, PATH_LOCKS_DIR,
                                       apr_pstrmemdup(pool, digest,
                                                      DIGEST_SUBDIR_LEN),
-                                      digest, NULL);
+                                      digest, (char *)NULL);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/libsvn_fs_x/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/transaction.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/transaction.c Thu Oct 24 15:00:12 2013
@@ -1045,7 +1045,7 @@ create_txn_dir(const char **id_p,
                                  PATH_TXNS_DIR,
                                  apr_pstrcat(pool, *id_p, PATH_EXT_TXN,
                                              (char *)NULL),
-                                 NULL);
+                                 (char *)NULL);
 
   return svn_io_dir_make(txn_dir, APR_OS_DEFAULT, pool);
 }

Modified: subversion/trunk/subversion/libsvn_fs_x/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/util.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/util.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/util.c Thu Oct 24 15:00:12 2013
@@ -141,7 +141,7 @@ svn_fs_x__path_rev_packed(svn_fs_t *fs, 
                               apr_psprintf(pool,
                                            "%ld" PATH_EXT_PACKED_SHARD,
                                            rev / ffd->max_files_per_dir),
-                              kind, NULL);
+                              kind, (char *)NULL);
 }
 
 const char *
@@ -151,7 +151,7 @@ svn_fs_x__path_rev_shard(svn_fs_t *fs, s
   return svn_dirent_join_many(pool, fs->path, PATH_REVS_DIR,
                               apr_psprintf(pool, "%ld",
                                                  rev / ffd->max_files_per_dir),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -202,7 +202,7 @@ svn_fs_x__path_revprops_shard(svn_fs_t *
   return svn_dirent_join_many(pool, fs->path, PATH_REVPROPS_DIR,
                               apr_psprintf(pool, "%ld",
                                            rev / ffd->max_files_per_dir),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -215,7 +215,7 @@ svn_fs_x__path_revprops_pack_shard(svn_f
   return svn_dirent_join_many(pool, fs->path, PATH_REVPROPS_DIR,
                               apr_psprintf(pool, "%ld" PATH_EXT_PACKED_SHARD,
                                            rev / ffd->max_files_per_dir),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -247,7 +247,7 @@ svn_fs_x__path_txn_dir(svn_fs_t *fs,
   return svn_dirent_join_many(pool, fs->path, PATH_TXNS_DIR,
                               combine_txn_id_string(txn_id, PATH_EXT_TXN,
                                                     pool),
-                              NULL);
+                              (char *)NULL);
 }
 
 /* Return the name of the sha1->rep mapping file in transaction TXN_ID
@@ -336,7 +336,7 @@ svn_fs_x__path_txn_proto_rev(svn_fs_t *f
   return svn_dirent_join_many(pool, fs->path, PATH_TXN_PROTOS_DIR,
                               combine_txn_id_string(txn_id, PATH_EXT_REV,
                                                     pool),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -348,7 +348,7 @@ svn_fs_x__path_txn_proto_rev_lock(svn_fs
                               combine_txn_id_string(txn_id,
                                                     PATH_EXT_REV_LOCK,
                                                     pool),
-                              NULL);
+                              (char *)NULL);
 }
 
 const char *
@@ -394,7 +394,7 @@ svn_fs_x__path_node_origin(svn_fs_t *fs,
     buffer[len - 1] = '\0';
 
   return svn_dirent_join_many(pool, fs->path, PATH_NODE_ORIGINS_DIR,
-                              buffer, NULL);
+                              buffer, (char *)NULL);
 }
 
 

Modified: subversion/trunk/subversion/libsvn_subr/config_file.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/config_file.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/config_file.c (original)
+++ subversion/trunk/subversion/libsvn_subr/config_file.c Thu Oct 24 15:00:12 2013
@@ -395,7 +395,8 @@ svn_config__sys_config_path(const char *
     const char *folder;
     SVN_ERR(svn_config__win_config_path(&folder, TRUE, pool));
     *path_p = svn_dirent_join_many(pool, folder,
-                                   SVN_CONFIG__SUBDIRECTORY, fname, NULL);
+                                   SVN_CONFIG__SUBDIRECTORY, fname,
+                                   (char *)NULL);
   }
 
 #elif defined(__HAIKU__)
@@ -408,11 +409,13 @@ svn_config__sys_config_path(const char *
       return SVN_NO_ERROR;
 
     *path_p = svn_dirent_join_many(pool, folder,
-                                   SVN_CONFIG__SYS_DIRECTORY, fname, NULL);
+                                   SVN_CONFIG__SYS_DIRECTORY, fname,
+                                   (char *)NULL);
   }
 #else  /* ! WIN32 && !__HAIKU__ */
 
-  *path_p = svn_dirent_join_many(pool, SVN_CONFIG__SYS_DIRECTORY, fname, NULL);
+  *path_p = svn_dirent_join_many(pool, SVN_CONFIG__SYS_DIRECTORY, fname,
+                                 (char *) NULL);
 
 #endif /* WIN32 */
 
@@ -1303,7 +1306,7 @@ svn_config_get_user_config_path(const ch
 
   if (config_dir)
     {
-      *path = svn_dirent_join_many(pool, config_dir, fname, NULL);
+      *path = svn_dirent_join_many(pool, config_dir, fname, (char *)NULL);
       return SVN_NO_ERROR;
     }
 
@@ -1312,7 +1315,7 @@ svn_config_get_user_config_path(const ch
     const char *folder;
     SVN_ERR(svn_config__win_config_path(&folder, FALSE, pool));
     *path = svn_dirent_join_many(pool, folder,
-                                 SVN_CONFIG__SUBDIRECTORY, fname, NULL);
+                                 SVN_CONFIG__SUBDIRECTORY, fname, (char *)NULL);
   }
 
 #elif defined(__HAIKU__)
@@ -1325,7 +1328,8 @@ svn_config_get_user_config_path(const ch
       return SVN_NO_ERROR;
 
     *path = svn_dirent_join_many(pool, folder,
-                                 SVN_CONFIG__USR_DIRECTORY, fname, NULL);
+                                 SVN_CONFIG__USR_DIRECTORY, fname,
+                                 (char *)NULL);
   }
 #else  /* ! WIN32 && !__HAIKU__ */
 
@@ -1335,7 +1339,7 @@ svn_config_get_user_config_path(const ch
       return SVN_NO_ERROR;
     *path = svn_dirent_join_many(pool,
                                svn_dirent_canonicalize(homedir, pool),
-                               SVN_CONFIG__USR_DIRECTORY, fname, NULL);
+                               SVN_CONFIG__USR_DIRECTORY, fname, (char*)NULL);
   }
 #endif /* WIN32 */
 

Modified: subversion/trunk/subversion/libsvn_wc/adm_files.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_files.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_files.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_files.c Thu Oct 24 15:00:12 2013
@@ -117,7 +117,7 @@ svn_wc__adm_child(const char *path,
                               path,
                               adm_dir_name,
                               child,
-                              NULL);
+                              (char *)NULL);
 }
 
 

Modified: subversion/trunk/subversion/libsvn_wc/upgrade.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/upgrade.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/upgrade.c (original)
+++ subversion/trunk/subversion/libsvn_wc/upgrade.c Thu Oct 24 15:00:12 2013
@@ -400,7 +400,7 @@ build_lockfile_path(const char *local_di
                               local_dir_abspath,
                               svn_wc_get_adm_dir(result_pool),
                               ADM_LOCK,
-                              NULL);
+                              (char *)NULL);
 }
 
 
@@ -1349,7 +1349,8 @@ bump_to_29(void *baton, svn_sqlite__db_t
   /* Rename all pristine files, adding a ".svn-base" suffix. */
   pristine_dir_abspath = svn_dirent_join_many(scratch_pool, wcroot_abspath,
                                               svn_wc_get_adm_dir(scratch_pool),
-                                              PRISTINE_STORAGE_RELPATH, NULL);
+                                              PRISTINE_STORAGE_RELPATH,
+                                              (char *)NULL);
   SVN_ERR(svn_io_dir_walk2(pristine_dir_abspath, APR_FINFO_MIN,
                            rename_pristine_file, NULL, scratch_pool));
 

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Oct 24 15:00:12 2013
@@ -13456,7 +13456,7 @@ svn_wc__db_temp_wcroot_tempdir(const cha
                                            wcroot->abspath,
                                            svn_wc_get_adm_dir(scratch_pool),
                                            WCROOT_TEMPDIR_RELPATH,
-                                           NULL);
+                                           (char *)NULL);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c Thu Oct 24 15:00:12 2013
@@ -67,7 +67,7 @@ get_pristine_fname(const char **pristine
                                           wcroot_abspath,
                                           svn_wc_get_adm_dir(scratch_pool),
                                           PRISTINE_STORAGE_RELPATH,
-                                          NULL);
+                                          (char *)NULL);
 
   /* We should have a valid checksum and (thus) a valid digest. */
   SVN_ERR_ASSERT(hexdigest != NULL);
@@ -85,7 +85,7 @@ get_pristine_fname(const char **pristine
                                            base_dir_abspath,
                                            subdir,
                                            hexdigest,
-                                           NULL);
+                                           (char *)NULL);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c (original)
+++ subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c Thu Oct 24 15:00:12 2013
@@ -363,7 +363,7 @@ get_access_conf(request_rec *r, authz_sv
         {
           access_file = svn_dirent_join_many(scratch_pool, repos_path, "conf",
                                              conf->repo_relative_access_file,
-                                             NULL);
+                                             (char *)NULL);
         }
     }
   else

Modified: subversion/trunk/subversion/tests/cmdline/entries-dump.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/entries-dump.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/entries-dump.c (original)
+++ subversion/trunk/subversion/tests/cmdline/entries-dump.c Thu Oct 24 15:00:12 2013
@@ -105,7 +105,7 @@ entries_dump(const char *dir_path, svn_w
       SVN_ERR(svn_wc__read_entries_old(&entries, dir_abspath, pool, pool));
       lockfile_path = svn_dirent_join_many(pool, dir_path,
                                            svn_wc_get_adm_dir(pool),
-                                           "lock", NULL);
+                                           "lock", (char *)NULL);
       SVN_ERR(svn_io_check_path(lockfile_path, &kind, pool));
       locked = (kind == svn_node_file);
     }

Modified: subversion/trunk/subversion/tests/libsvn_client/client-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/client-test.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/client-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/client-test.c Thu Oct 24 15:00:12 2013
@@ -390,7 +390,8 @@ test_patch(const svn_test_opts_t *opts,
 
   /* Create the patch file. */
   patch_file_path = svn_dirent_join_many(pool, cwd,
-                                         "test-patch", "test-patch.diff", NULL);
+                                         "test-patch", "test-patch.diff",
+                                         (char *)NULL);
   SVN_ERR(svn_io_file_open(&patch_file, patch_file_path,
                            (APR_READ | APR_WRITE | APR_CREATE | APR_TRUNCATE),
                            APR_OS_DEFAULT, pool));

Modified: subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c Thu Oct 24 15:00:12 2013
@@ -304,7 +304,7 @@ pack_filesystem(const svn_test_opts_t *o
     {
       path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                   apr_psprintf(pool, "%d.pack", i / SHARD_SIZE),
-                                  "pack", NULL);
+                                  "pack", (char *)NULL);
 
       /* These files should exist. */
       SVN_ERR(svn_io_check_path(path, &kind, pool));
@@ -314,7 +314,7 @@ pack_filesystem(const svn_test_opts_t *o
 
       path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                   apr_psprintf(pool, "%d.pack", i / SHARD_SIZE),
-                                  "manifest", NULL);
+                                  "manifest", (char *)NULL);
       SVN_ERR(svn_io_check_path(path, &kind, pool));
       if (kind != svn_node_file)
         return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -324,7 +324,7 @@ pack_filesystem(const svn_test_opts_t *o
       /* This directory should not exist. */
       path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                   apr_psprintf(pool, "%d", i / SHARD_SIZE),
-                                  NULL);
+                                  (char *)NULL);
       SVN_ERR(svn_io_check_path(path, &kind, pool));
       if (kind != svn_node_none)
         return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -346,7 +346,7 @@ pack_filesystem(const svn_test_opts_t *o
   /* Finally, make sure the final revision directory does exist. */
   path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                               apr_psprintf(pool, "%d", (i / SHARD_SIZE) + 1),
-                              NULL);
+                              (char *)NULL);
   SVN_ERR(svn_io_check_path(path, &kind, pool));
   if (kind != svn_node_none)
     return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -378,7 +378,7 @@ pack_even_filesystem(const svn_test_opts
   SVN_ERR(create_packed_filesystem(REPO_NAME, opts, MAX_REV, SHARD_SIZE,
                                    pool));
 
-  path = svn_dirent_join_many(pool, REPO_NAME, "revs", "2.pack", NULL);
+  path = svn_dirent_join_many(pool, REPO_NAME, "revs", "2.pack", (char *)NULL);
   SVN_ERR(svn_io_check_path(path, &kind, pool));
   if (kind != svn_node_dir)
     return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -692,7 +692,7 @@ recover_fully_packed(const svn_test_opts
                                    apr_psprintf(pool, "%ld/%ld",
                                                 after_rev / SHARD_SIZE,
                                                 after_rev),
-                                   NULL),
+                                   (char *)NULL),
               FALSE, pool));
   err = svn_fs_recover(REPO_NAME, NULL, NULL, pool);
   if (! err)

Modified: subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c Thu Oct 24 15:00:12 2013
@@ -288,7 +288,7 @@ pack_filesystem(const svn_test_opts_t *o
     {
       path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                   apr_psprintf(pool, "%d.pack", i / SHARD_SIZE),
-                                  "pack", NULL);
+                                  "pack", (char *)NULL);
 
       /* These files should exist. */
       SVN_ERR(svn_io_check_path(path, &kind, pool));
@@ -300,7 +300,7 @@ pack_filesystem(const svn_test_opts_t *o
         {
           path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                       apr_psprintf(pool, "%d.pack", i / SHARD_SIZE),
-                                      "manifest", NULL);
+                                      "manifest", (char *)NULL);
           SVN_ERR(svn_io_check_path(path, &kind, pool));
           if (kind != svn_node_file)
             return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -311,7 +311,7 @@ pack_filesystem(const svn_test_opts_t *o
         {
           path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                       apr_psprintf(pool, "%d.pack", i / SHARD_SIZE),
-                                      "pack.l2p", NULL);
+                                      "pack.l2p", (char *)NULL);
           SVN_ERR(svn_io_check_path(path, &kind, pool));
           if (kind != svn_node_file)
             return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -320,7 +320,7 @@ pack_filesystem(const svn_test_opts_t *o
 
           path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                       apr_psprintf(pool, "%d.pack", i / SHARD_SIZE),
-                                      "pack.p2l", NULL);
+                                      "pack.p2l", (char *)NULL);
           SVN_ERR(svn_io_check_path(path, &kind, pool));
           if (kind != svn_node_file)
             return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -331,7 +331,7 @@ pack_filesystem(const svn_test_opts_t *o
       /* This directory should not exist. */
       path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                                   apr_psprintf(pool, "%d", i / SHARD_SIZE),
-                                  NULL);
+                                  (char *)NULL);
       SVN_ERR(svn_io_check_path(path, &kind, pool));
       if (kind != svn_node_none)
         return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -353,7 +353,7 @@ pack_filesystem(const svn_test_opts_t *o
   /* Finally, make sure the final revision directory does exist. */
   path = svn_dirent_join_many(pool, REPO_NAME, "revs",
                               apr_psprintf(pool, "%d", (i / SHARD_SIZE) + 1),
-                              NULL);
+                              (char *)NULL);
   SVN_ERR(svn_io_check_path(path, &kind, pool));
   if (kind != svn_node_none)
     return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -385,7 +385,7 @@ pack_even_filesystem(const svn_test_opts
   SVN_ERR(create_packed_filesystem(REPO_NAME, opts, MAX_REV, SHARD_SIZE,
                                    pool));
 
-  path = svn_dirent_join_many(pool, REPO_NAME, "revs", "2.pack", NULL);
+  path = svn_dirent_join_many(pool, REPO_NAME, "revs", "2.pack", (char *)NULL);
   SVN_ERR(svn_io_check_path(path, &kind, pool));
   if (kind != svn_node_dir)
     return svn_error_createf(SVN_ERR_FS_GENERAL, NULL,
@@ -724,7 +724,7 @@ recover_fully_packed(const svn_test_opts
                                    apr_psprintf(pool, "%ld/%ld",
                                                 after_rev / SHARD_SIZE,
                                                 after_rev),
-                                   NULL),
+                                   (char *)NULL),
               FALSE, pool));
   err = svn_fs_recover(REPO_NAME, NULL, NULL, pool);
   if (! err)

Modified: subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Thu Oct 24 15:00:12 2013
@@ -269,7 +269,7 @@ test_dirent_join(apr_pool_t *pool)
                                  "\"%s\". expected \"%s\"",
                                  base, comp, result, expect);
 
-      result = svn_dirent_join_many(pool, base, comp, NULL);
+      result = svn_dirent_join_many(pool, base, comp, (char *)NULL);
       if (strcmp(result, expect))
         return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
                                  "svn_dirent_join_many(\"%s\", \"%s\") returned "

Modified: subversion/trunk/subversion/tests/libsvn_subr/path-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/path-test.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/path-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/path-test.c Thu Oct 24 15:00:12 2013
@@ -489,7 +489,7 @@ test_path_join(apr_pool_t *pool)
       if (svn_path_is_url(base))
         continue;
 
-      result = svn_path_join_many(pool, base, comp, NULL);
+      result = svn_path_join_many(pool, base, comp, (char *)NULL);
       if (strcmp(result, expect))
         return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
                                  "svn_path_join_many(\"%s\", \"%s\") returned "

Modified: subversion/trunk/subversion/tests/libsvn_wc/entries-compat.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/entries-compat.c?rev=1535399&r1=1535398&r2=1535399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/entries-compat.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/entries-compat.c Thu Oct 24 15:00:12 2013
@@ -377,7 +377,7 @@ test_entries_alloc(apr_pool_t *pool)
                                        "fake-wc",
                                        WC_NAME,
                                        "D",
-                                       NULL);
+                                       (char *)NULL);
   SVN_ERR(svn_wc_entry(&entry, local_relpath, adm_access, TRUE, pool));
   SVN_TEST_ASSERT(entry == apr_hash_get(entries, "D", APR_HASH_KEY_STRING));
 
@@ -413,7 +413,7 @@ test_stubs(apr_pool_t *pool)
                                        "fake-wc",
                                        WC_NAME,
                                        "M",
-                                       NULL);
+                                       (char *)NULL);
 
   SVN_ERR(svn_wc_adm_open3(&adm_access,
                            NULL /* associated */,



Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Philip Martin <ph...@wandisco.com>.
Alan Barrett <ap...@cequrux.com> writes:

> On Fri, 25 Oct 2013, Branko Čibej wrote:
>>> URL: http://svn.apache.org/r1535399
>>> Log:
>>> Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>>> and svn_path_join_many.
>>
>>
>>Why? On which platform do you expect a char* to have a different size
>>and/or representation than a void*?
>
> NULL may be defined as any null pointer constant [1]; acceptable values include 0 or ((void *)0) [2].  If NULL is defined as 0, then passing it to a varargs function that expects any kind of pointer will not work properly.

We have had this discussion in the past and decided that casting was
strictly correct; in some places we were already using (char *)NULL:

http://svn.haxx.se/dev/archive-2011-02/0841.shtml

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Alan Barrett <ap...@cequrux.com>.
On Fri, 25 Oct 2013, Julian Foad wrote:
>> On Fri, 25 Oct 2013, Branko Čibej wrote:
>
>>>>  URL: http://svn.apache.org/r1535399
>>>>  Log:
>>>>  Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>>>>  and svn_path_join_many.
>>>
>>>  Why? On which platform do you expect a char* to have a different size
>>>  and/or representation than a void*?
>
>Alan Barrett wrote:
>> [theory]
>
>Alan, Brane specifically asked about practice not theory.

I was trying to point out that the "On which platform ..." 
question appeared to be based on the incorrect assumption that 
NULL macro would always expand to a void*.  Asking about the 
relationship between char* and void* is the wrong question.

--apb (Alan Barrett)

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Branko Čibej <br...@wandisco.com>.
On 25.10.2013 11:50, Branko Čibej wrote:
> On 25.10.2013 11:40, Philip Martin wrote:
>> Branko Čibej <br...@wandisco.com> writes:
>>
>>> Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
>>> should be documented in HACKING, and (b) there should be a '#define
>>> NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
>>> svn_private_config.h. I really do not want to see '(char*)NULL' all over
>>> the place. That's ... unsightly.
>> That would be incompatible with C++ since (void*) doesn't automatically
>> convert to (char*) by default.
> You do realize that this is entirely irrelevant for varargs, don't you?
>
>>   I'd like to avoid such incompatibilities
>> because I'd like to be able to use a C++ compiler at some point to catch
>> things like enum mismatches.  I suppose we could have NULLCHARPTR for
>> variadic functions expecting (char*) which is the common case.
> For all I care it can be ((intptr_t)0) which will always be the correct
> size and can always be converted to a null pointer constant. But I'm
> also pretty sure that we definitely do not want to use this special
> constant in anything but varargs. Maybe call it VA_NULL, to make the
> intent more mnemonic.

Even better:

#define VA_NULL ((struct svn_fake_and_never_defined*)0)

You can put this into a variable argument list, but not assign it to a
pointer variable or argument of a known type.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Philip Martin <ph...@wandisco.com>.
Branko Čibej <br...@wandisco.com> writes:

> On 25.10.2013 11:40, Philip Martin wrote:
>> Branko Čibej <br...@wandisco.com> writes:
>>
>>> Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
>>> should be documented in HACKING, and (b) there should be a '#define
>>> NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
>>> svn_private_config.h. I really do not want to see '(char*)NULL' all over
>>> the place. That's ... unsightly.
>> That would be incompatible with C++ since (void*) doesn't automatically
>> convert to (char*) by default.
>
> You do realize that this is entirely irrelevant for varargs, don't you?

Ah, yes, that definition of NULLPTR would work for varargs.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Branko Čibej <br...@wandisco.com>.
On 25.10.2013 11:40, Philip Martin wrote:
> Branko Čibej <br...@wandisco.com> writes:
>
>> Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
>> should be documented in HACKING, and (b) there should be a '#define
>> NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
>> svn_private_config.h. I really do not want to see '(char*)NULL' all over
>> the place. That's ... unsightly.
> That would be incompatible with C++ since (void*) doesn't automatically
> convert to (char*) by default.

You do realize that this is entirely irrelevant for varargs, don't you?

>   I'd like to avoid such incompatibilities
> because I'd like to be able to use a C++ compiler at some point to catch
> things like enum mismatches.  I suppose we could have NULLCHARPTR for
> variadic functions expecting (char*) which is the common case.

For all I care it can be ((intptr_t)0) which will always be the correct
size and can always be converted to a null pointer constant. But I'm
also pretty sure that we definitely do not want to use this special
constant in anything but varargs. Maybe call it VA_NULL, to make the
intent more mnemonic.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:

> I agree that I don't like the casts there. I don't think any of the
> supported architectures has a problem with the original code.

I don't mind too much if somebody wants to change to using NULL
everwhere.  My primary concern was the inconsistency: we had decided
that the cast was strictly correct and we were using it in some places,
but other places had no cast.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

RE: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Philip Martin [mailto:philip.martin@wandisco.com]
> Sent: vrijdag 25 oktober 2013 11:41
> To: Branko Čibej
> Cc: Subversion Development
> Subject: Re: svn commit: r1535399 - in /subversion/trunk/subversion:
> libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/
> tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/
> tests/libsvn_subr/ tests/libsvn_wc/
> 
> Branko Čibej <br...@wandisco.com> writes:
> 
> > Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
> > should be documented in HACKING, and (b) there should be a '#define
> > NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
> > svn_private_config.h. I really do not want to see '(char*)NULL' all over
> > the place. That's ... unsightly.
> 
> That would be incompatible with C++ since (void*) doesn't automatically
> convert to (char*) by default.  I'd like to avoid such incompatibilities
> because I'd like to be able to use a C++ compiler at some point to catch
> things like enum mismatches.  I suppose we could have NULLCHARPTR for
> variadic functions expecting (char*) which is the common case.

Our C++ code can just use NULL as it does today, while our C code uses a NULLPTR defined in a private header.

I agree that I don't like the casts there. I don't think any of the supported architectures has a problem with the original code. 

On Windows where int and long are always 4 byte in x64 mode, the varargs on x64 always use 8 bytes to transfer integer types to avoid the problems we are talking about here. So as far as I can tell we are only looking at some gcc warning that we somehow enable, and want to suppress.


In the swig bindings there is a far more serious problem where we convert function pointers to void* and back, which is illegal according to the spec. (Gives hundreds if not thousands of warnings on the Windows build). If we really want to spend time on these issues we should look at those instead.

Luckily the same thing applies here: I don't know of currently supported architectures that care about this problem.

	Bert


Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Philip Martin <ph...@wandisco.com>.
Branko Čibej <br...@wandisco.com> writes:

> Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
> should be documented in HACKING, and (b) there should be a '#define
> NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
> svn_private_config.h. I really do not want to see '(char*)NULL' all over
> the place. That's ... unsightly.

That would be incompatible with C++ since (void*) doesn't automatically
convert to (char*) by default.  I'd like to avoid such incompatibilities
because I'd like to be able to use a C++ compiler at some point to catch
things like enum mismatches.  I suppose we could have NULLCHARPTR for
variadic functions expecting (char*) which is the common case.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 25 October 2013 13:28, Branko Čibej <br...@wandisco.com> wrote:
> On 25.10.2013 11:18, Branko Čibej wrote:
>> On 25.10.2013 11:15, Julian Foad wrote:
>>> I (Julian Foad) wrote:
>>>
>>>>>  On Fri, 25 Oct 2013, Branko Čibej wrote:
>>>>>>>   URL: http://svn.apache.org/r1535399
>>>>>>>   Log:
>>>>>>>   Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>>>>>>>   and svn_path_join_many.
>>>>>>   Why? On which platform do you expect a char* to have a different size
>>>>>>   and/or representation than a void*?
>>>> Brane, we have been doing this consistently for the last three years;
>>>> Philip's commit was just continuing it.  See [1] for the original
>>>> discussion.
>>>>
>>>> [1] Email to dev@ from Stefan Sperling on 2011-02-25, subject "Re: svn
>>>> commit: r1074461 ...",
>>>> <http://svn.haxx.se/dev/archive-2011-02/0841.shtml>.
>>> Now having read the original discussion (October 2010; the later email mentioned above is merely a good reference to it), it appears we are doing this in order to silence compiler warnings rather than for object-code correctness.
>> Yes, exactly my point ... compiler warnings that someone goes out of
>> their way to enable, even though they're not enabled in maintainer mode.
>> You know how I feel about /that/.
>>
>> And then we have week-long flames on the dev@ list when someone
>> pre-emptively initializes a local var to silence a "may be used
>> uninitialized" warning. See the first entry in the 1.8.x STATUS file,
>> for example. Duh.
>
> Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
> should be documented in HACKING, and (b) there should be a '#define
> NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
> svn_private_config.h. I really do not want to see '(char*)NULL' all over
> the place. That's ... unsightly.
>
> Oh, and (c) the associated warning must be enabled in maintainer mode.
>
+1 on all points!


-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Branko Čibej <br...@wandisco.com>.
On 25.10.2013 11:18, Branko Čibej wrote:
> On 25.10.2013 11:15, Julian Foad wrote:
>> I (Julian Foad) wrote:
>>
>>>>  On Fri, 25 Oct 2013, Branko Čibej wrote:
>>>>>>   URL: http://svn.apache.org/r1535399
>>>>>>   Log:
>>>>>>   Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>>>>>>   and svn_path_join_many.
>>>>>   Why? On which platform do you expect a char* to have a different size
>>>>>   and/or representation than a void*?
>>> Brane, we have been doing this consistently for the last three years; 
>>> Philip's commit was just continuing it.  See [1] for the original 
>>> discussion.
>>>
>>> [1] Email to dev@ from Stefan Sperling on 2011-02-25, subject "Re: svn 
>>> commit: r1074461 ...", 
>>> <http://svn.haxx.se/dev/archive-2011-02/0841.shtml>.
>> Now having read the original discussion (October 2010; the later email mentioned above is merely a good reference to it), it appears we are doing this in order to silence compiler warnings rather than for object-code correctness.
> Yes, exactly my point ... compiler warnings that someone goes out of
> their way to enable, even though they're not enabled in maintainer mode.
> You know how I feel about /that/.
>
> And then we have week-long flames on the dev@ list when someone
> pre-emptively initializes a local var to silence a "may be used
> uninitialized" warning. See the first entry in the 1.8.x STATUS file,
> for example. Duh.

Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
should be documented in HACKING, and (b) there should be a '#define
NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
svn_private_config.h. I really do not want to see '(char*)NULL' all over
the place. That's ... unsightly.

Oh, and (c) the associated warning must be enabled in maintainer mode.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Branko Čibej <br...@wandisco.com>.
On 25.10.2013 11:15, Julian Foad wrote:
> I (Julian Foad) wrote:
>
>>>  On Fri, 25 Oct 2013, Branko Čibej wrote:
>>>>>   URL: http://svn.apache.org/r1535399
>>>>>   Log:
>>>>>   Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>>>>>   and svn_path_join_many.
>>>>   Why? On which platform do you expect a char* to have a different size
>>>>   and/or representation than a void*?
>> Brane, we have been doing this consistently for the last three years; 
>> Philip's commit was just continuing it.  See [1] for the original 
>> discussion.
>>
>> [1] Email to dev@ from Stefan Sperling on 2011-02-25, subject "Re: svn 
>> commit: r1074461 ...", 
>> <http://svn.haxx.se/dev/archive-2011-02/0841.shtml>.
> Now having read the original discussion (October 2010; the later email mentioned above is merely a good reference to it), it appears we are doing this in order to silence compiler warnings rather than for object-code correctness.

Yes, exactly my point ... compiler warnings that someone goes out of
their way to enable, even though they're not enabled in maintainer mode.
You know how I feel about /that/.

And then we have week-long flames on the dev@ list when someone
pre-emptively initializes a local var to silence a "may be used
uninitialized" warning. See the first entry in the 1.8.x STATUS file,
for example. Duh.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Julian Foad <ju...@btopenworld.com>.
I (Julian Foad) wrote:

>>  On Fri, 25 Oct 2013, Branko Čibej wrote:
>>>>   URL: http://svn.apache.org/r1535399
>>>>   Log:
>>>>   Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>>>>   and svn_path_join_many.
>>> 
>>>   Why? On which platform do you expect a char* to have a different size
>>>   and/or representation than a void*?
> 
> Brane, we have been doing this consistently for the last three years; 
> Philip's commit was just continuing it.  See [1] for the original 
> discussion.
> 
> [1] Email to dev@ from Stefan Sperling on 2011-02-25, subject "Re: svn 
> commit: r1074461 ...", 
> <http://svn.haxx.se/dev/archive-2011-02/0841.shtml>.

Now having read the original discussion (October 2010; the later email mentioned above is merely a good reference to it), it appears we are doing this in order to silence compiler warnings rather than for object-code correctness.

- Julian

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Julian Foad <ju...@btopenworld.com>.
> On Fri, 25 Oct 2013, Branko Čibej wrote:

>>>  URL: http://svn.apache.org/r1535399
>>>  Log:
>>>  Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>>>  and svn_path_join_many.
>> 
>>  Why? On which platform do you expect a char* to have a different size
>>  and/or representation than a void*?

Alan Barrett wrote:
> [theory]

Alan, Brane specifically asked about practice not theory.

Brane, we have been doing this consistently for the last three years; Philip's commit was just continuing it.  See [1] for the original discussion.

[1] Email to dev@ from Stefan Sperling on 2011-02-25, subject "Re: svn commit: r1074461 ...", <http://svn.haxx.se/dev/archive-2011-02/0841.shtml>.

- Julian

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Alan Barrett <ap...@cequrux.com>.
On Fri, 25 Oct 2013, Branko Čibej wrote:
>> URL: http://svn.apache.org/r1535399
>> Log:
>> Cast NULL to (char *) in vararg calls to svn_dirent_join_many
>> and svn_path_join_many.
>
>
>Why? On which platform do you expect a char* to have a different size
>and/or representation than a void*?

NULL may be defined as any null pointer constant [1]; acceptable 
values include 0 or ((void *)0) [2].  If NULL is defined as 0, 
then passing it to a varargs function that expects any kind of 
pointer will not work properly.

[1] ISO 9899:1999 section 7.17 paragraph 3: "The macros are NULL 
which expands to an implementation-defined null pointer constant; 
and ..."

[2] ISO 9899:1999 section 6.3.2.3 paragraph 1: "An integer 
constant expression with the value 0, or such an expression cast 
to type void*, is called a null pointer constant."

--apb (Alan Barrett)

Re: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Branko Čibej <br...@wandisco.com>.
On 24.10.2013 17:00, philip@apache.org wrote:
> Author: philip
> Date: Thu Oct 24 15:00:12 2013
> New Revision: 1535399
>
> URL: http://svn.apache.org/r1535399
> Log:
> Cast NULL to (char *) in vararg calls to svn_dirent_join_many
> and svn_path_join_many.


Why? On which platform do you expect a char* to have a different size
and/or representation than a void*? If you can't point to a specific
crash caused by passing NULL instead of (char*)NULL, then you've messed
up the diffs for no good reason.

-- Brane

P.S.: I'm well aware that, according to a strict interpretation of the C
standard, (void*)0 and (char*)0 are not interchangeable. But we do not
support a single platform where the ABI does not treat them identically.
Which is why I throw a fit every time I see someone casting (void*) to
something else (in C, not C++), or worse, the other way around -- other
than to add or remove const.


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com