> From: H. Hernan Moraldo <games@moraldo.com.ar>
>
> El mié, 10-03-2004 a las 14:11, Julian Foad escribió:
>
>> H. Hernan Moraldo wrote:
>> > Remove the #include <stdlib.h> line from svn_types.h, and add that line
>> > to any other files that actually need it.
>> [...]
>> > On subversion/include/svn_types.h there were some lines telling:
>> > > /* ### this should go away, but it causes too much breakage right now */
>> > #include <stdlib.h>
>>
>> Very nearly OK ... but there is one macro that needs stdlib.h:
>>
>> /** Convert null-terminated C string @a str to a revision number. */
>> #define SVN_STR_TO_REV(str) ((svn_revnum_t) atol(str))
>
> According to the replies, I think there are three choices:
>
> 1- To keep it all just as it is now
>
> 2- To covert SVN_STR_TO_REV to a function, and that would be major
> breakage for real, just as pointed in the original commentary. If you
> say this is the preferred option, I can try working on this patch.
Ideally this should be done, but I see two major issues. First, in which library would the implementation live, and would that meet our compatibility guarantees? Second, if we replace this function-like macro with a true function of the same name, would we consider that to be source-code-compatible enough, and would the same (all-capitals) name be aesthetically acceptable? (I am not directly asking you these questions, just saying that they would have to be answered if we try to do this.)
> 3- The third option is the one drafted on the patch attached: we don't
> remove stdlib from svn_types.h, but we stop assuming it is included
> automatically by including svn_types.h on the other source files. I
> think this makes for better code, but couldn't know if you agree on
> this.
Yes, I think that is a good thing to do and makes for better code. If a source file is directly using facilities from some header, it should directly include that header. This applies not just for <stdlib.h>, but to all headers. In fact, I have already started doing this for Subversion headers. I wrote the script attached ("includes.sh") to help find missing and redundant "#include" lines. It is by no means comprehensive, but it does find certain cases. The commented out code finds those cases where the header file is a nice tidy one with all declearations having the same prefix that matches the header file's name.
The results that I have obtained so far are in the second attachment ("tidy-includes.patch"). I stopped working on this because it didn't really seem worthwhile to do half of the job. If I can't find all of the missing and redundant includes, then I am not sure whether it is really worth fixing any of them. Maybe it is.
Options 2 and 3 are not mutually exclusive, of course. Both of them are good things to do, in principle. In practice, I don't know.
- Julian
Remove unnecessary #includes.
* subversion/libsvn_wc/props.c (###?)
* subversion/libsvn_wc/translate.c
* subversion/clients/cmdline/add-cmd.c
* subversion/clients/cmdline/blame-cmd.c
* subversion/clients/cmdline/checkout-cmd.c
* subversion/clients/cmdline/cleanup-cmd.c
* subversion/clients/cmdline/copy-cmd.c
* subversion/clients/cmdline/delete-cmd.c
* subversion/clients/cmdline/diff-cmd.c
* subversion/clients/cmdline/export-cmd.c
* subversion/clients/cmdline/help-cmd.c
* subversion/clients/cmdline/import-cmd.c
* subversion/clients/cmdline/log-cmd.c
* subversion/clients/cmdline/merge-cmd.c
* subversion/clients/cmdline/mkdir-cmd.c
* subversion/clients/cmdline/move-cmd.c
* subversion/clients/cmdline/notify.c
* subversion/clients/cmdline/prompt.c
* subversion/clients/cmdline/propdel-cmd.c
* subversion/clients/cmdline/propget-cmd.c
* subversion/clients/cmdline/proplist-cmd.c
* subversion/clients/cmdline/props.c
* subversion/clients/cmdline/propset-cmd.c
* subversion/clients/cmdline/resolved-cmd.c
* subversion/clients/cmdline/revert-cmd.c
* subversion/clients/cmdline/status-cmd.c
* subversion/clients/cmdline/update-cmd.c
* subversion/clients/cmdline/util.c
Do not include headers that are not used.
Include headers that are used directly but were only included indirectly.
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 8683)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -18,19 +18,10 @@
-#include <stdio.h> /* temporary, for printf() */
-#include <stdlib.h>
#include <string.h>
-#include <assert.h>
#include <apr_pools.h>
#include <apr_hash.h>
-#include <apr_tables.h>
-#include <apr_file_io.h>
-#include <apr_strings.h>
-#include <apr_lib.h>
-#include <apr_general.h>
#include "svn_types.h"
-#include "svn_delta.h"
#include "svn_string.h"
#include "svn_pools.h"
#include "svn_path.h"
@@ -40,7 +31,6 @@
#include "svn_io.h"
#include "svn_hash.h"
#include "svn_wc.h"
-#include "svn_time.h"
#include "svn_utf.h"
#include "wc.h"
Index: subversion/libsvn_wc/translate.c
===================================================================
--- subversion/libsvn_wc/translate.c (revision 8683)
+++ subversion/libsvn_wc/translate.c (working copy)
@@ -21,13 +21,9 @@
#include <stdlib.h>
#include <string.h>
#include <assert.h>
-#include <apr_general.h> /* for strcasecmp() */
#include <apr_pools.h>
-#include <apr_hash.h>
-#include <apr_tables.h>
#include <apr_file_io.h>
#include <apr_strings.h>
-#include <apr_lib.h>
#include "svn_types.h"
#include "svn_delta.h"
#include "svn_string.h"
Index: subversion/clients/cmdline/merge-cmd.c
===================================================================
--- subversion/clients/cmdline/merge-cmd.c (revision 8683)
+++ subversion/clients/cmdline/merge-cmd.c (working copy)
@@ -22,7 +22,6 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_path.h"
Index: subversion/clients/cmdline/props.c
===================================================================
--- subversion/clients/cmdline/props.c (revision 8683)
+++ subversion/clients/cmdline/props.c (working copy)
@@ -24,10 +24,7 @@
#include <apr_hash.h>
#include "svn_cmdline.h"
-#include "svn_wc.h"
-#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_delta.h"
#include "svn_error.h"
#include "svn_subst.h"
Index: subversion/clients/cmdline/checkout-cmd.c
===================================================================
--- subversion/clients/cmdline/checkout-cmd.c (revision 8683)
+++ subversion/clients/cmdline/checkout-cmd.c (working copy)
@@ -22,7 +22,6 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_path.h"
Index: subversion/clients/cmdline/propdel-cmd.c
===================================================================
--- subversion/clients/cmdline/propdel-cmd.c (revision 8683)
+++ subversion/clients/cmdline/propdel-cmd.c (working copy)
@@ -23,7 +23,6 @@
/*** Includes. ***/
#include "svn_cmdline.h"
-#include "svn_wc.h"
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_string.h"
Index: subversion/clients/cmdline/mkdir-cmd.c
===================================================================
--- subversion/clients/cmdline/mkdir-cmd.c (revision 8683)
+++ subversion/clients/cmdline/mkdir-cmd.c (working copy)
@@ -22,11 +22,9 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_delta.h"
#include "svn_error.h"
#include "cl.h"
Index: subversion/clients/cmdline/move-cmd.c
===================================================================
--- subversion/clients/cmdline/move-cmd.c (revision 8683)
+++ subversion/clients/cmdline/move-cmd.c (working copy)
@@ -22,10 +22,8 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_delta.h"
#include "svn_error.h"
#include "cl.h"
Index: subversion/clients/cmdline/revert-cmd.c
===================================================================
--- subversion/clients/cmdline/revert-cmd.c (revision 8683)
+++ subversion/clients/cmdline/revert-cmd.c (working copy)
@@ -24,7 +24,6 @@
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_pools.h"
#include "svn_error.h"
#include "cl.h"
Index: subversion/clients/cmdline/diff-cmd.c
===================================================================
--- subversion/clients/cmdline/diff-cmd.c (revision 8683)
+++ subversion/clients/cmdline/diff-cmd.c (working copy)
@@ -22,7 +22,6 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_string.h"
Index: subversion/clients/cmdline/copy-cmd.c
===================================================================
--- subversion/clients/cmdline/copy-cmd.c (revision 8683)
+++ subversion/clients/cmdline/copy-cmd.c (working copy)
@@ -22,7 +22,6 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_path.h"
Index: subversion/clients/cmdline/util.c
===================================================================
--- subversion/clients/cmdline/util.c (revision 8683)
+++ subversion/clients/cmdline/util.c (working copy)
@@ -33,7 +33,6 @@
#include <apr_general.h>
#include <apr_lib.h>
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_path.h"
Index: subversion/clients/cmdline/propget-cmd.c
===================================================================
--- subversion/clients/cmdline/propget-cmd.c (revision 8683)
+++ subversion/clients/cmdline/propget-cmd.c (working copy)
@@ -24,7 +24,6 @@
#include "svn_cmdline.h"
#include "svn_pools.h"
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_delta.h"
Index: subversion/clients/cmdline/prompt.c
===================================================================
--- subversion/clients/cmdline/prompt.c (revision 8683)
+++ subversion/clients/cmdline/prompt.c (working copy)
@@ -25,8 +25,6 @@
#include <apr_lib.h>
#include "svn_cmdline.h"
-#include "svn_wc.h"
-#include "svn_client.h"
#include "svn_string.h"
#include "svn_delta.h"
#include "svn_auth.h"
Index: subversion/clients/cmdline/log-cmd.c
===================================================================
--- subversion/clients/cmdline/log-cmd.c (revision 8683)
+++ subversion/clients/cmdline/log-cmd.c (working copy)
@@ -25,7 +25,6 @@
#define APR_WANT_STRFUNC
#include <apr_want.h>
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_path.h"
Index: subversion/clients/cmdline/update-cmd.c
===================================================================
--- subversion/clients/cmdline/update-cmd.c (revision 8683)
+++ subversion/clients/cmdline/update-cmd.c (working copy)
@@ -22,7 +22,6 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_string.h"
Index: subversion/clients/cmdline/resolved-cmd.c
===================================================================
--- subversion/clients/cmdline/resolved-cmd.c (revision 8683)
+++ subversion/clients/cmdline/resolved-cmd.c (working copy)
@@ -24,10 +24,8 @@
#define APR_WANT_STDIO
#include <apr_want.h>
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_delta.h"
#include "svn_error.h"
#include "svn_pools.h"
Index: subversion/clients/cmdline/cleanup-cmd.c
===================================================================
--- subversion/clients/cmdline/cleanup-cmd.c (revision 8683)
+++ subversion/clients/cmdline/cleanup-cmd.c (working copy)
@@ -24,7 +24,6 @@
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_pools.h"
#include "svn_error.h"
#include "cl.h"
Index: subversion/clients/cmdline/add-cmd.c
===================================================================
--- subversion/clients/cmdline/add-cmd.c (revision 8683)
+++ subversion/clients/cmdline/add-cmd.c (working copy)
@@ -24,10 +24,8 @@
#define APR_WANT_STDIO
#include <apr_want.h>
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_delta.h"
#include "svn_error.h"
#include "svn_pools.h"
Index: subversion/clients/cmdline/help-cmd.c
===================================================================
--- subversion/clients/cmdline/help-cmd.c (revision 8683)
+++ subversion/clients/cmdline/help-cmd.c (working copy)
@@ -22,11 +22,9 @@
/*** Includes. ***/
-#include "svn_client.h"
#include "svn_string.h"
#include "svn_error.h"
-#include "svn_version.h"
-#include "svn_utf.h"
+#include "svn_ra.h"
#include "cl.h"
Index: subversion/clients/cmdline/propset-cmd.c
===================================================================
--- subversion/clients/cmdline/propset-cmd.c (revision 8683)
+++ subversion/clients/cmdline/propset-cmd.c (working copy)
@@ -23,7 +23,6 @@
/*** Includes. ***/
#include "svn_cmdline.h"
-#include "svn_wc.h"
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_string.h"
Index: subversion/clients/cmdline/delete-cmd.c
===================================================================
--- subversion/clients/cmdline/delete-cmd.c (revision 8683)
+++ subversion/clients/cmdline/delete-cmd.c (working copy)
@@ -22,10 +22,8 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_delta.h"
#include "svn_error.h"
#include "svn_pools.h"
Index: subversion/clients/cmdline/notify.c
===================================================================
--- subversion/clients/cmdline/notify.c (revision 8683)
+++ subversion/clients/cmdline/notify.c (working copy)
@@ -30,6 +30,7 @@
#include "svn_cmdline.h"
#include "svn_pools.h"
+#include "svn_wc.h"
#include "cl.h"
Index: subversion/clients/cmdline/import-cmd.c
===================================================================
--- subversion/clients/cmdline/import-cmd.c (revision 8683)
+++ subversion/clients/cmdline/import-cmd.c (working copy)
@@ -22,7 +22,6 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_path.h"
Index: subversion/clients/cmdline/proplist-cmd.c
===================================================================
--- subversion/clients/cmdline/proplist-cmd.c (revision 8683)
+++ subversion/clients/cmdline/proplist-cmd.c (working copy)
@@ -23,7 +23,6 @@
/*** Includes. ***/
#include "svn_cmdline.h"
-#include "svn_wc.h"
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_string.h"
Index: subversion/clients/cmdline/export-cmd.c
===================================================================
--- subversion/clients/cmdline/export-cmd.c (revision 8683)
+++ subversion/clients/cmdline/export-cmd.c (working copy)
@@ -22,7 +22,6 @@
/*** Includes. ***/
-#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
#include "svn_error.h"
Index: subversion/clients/cmdline/status-cmd.c
===================================================================
--- subversion/clients/cmdline/status-cmd.c (revision 8683)
+++ subversion/clients/cmdline/status-cmd.c (working copy)
@@ -25,7 +25,6 @@
#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
-#include "svn_path.h"
#include "svn_delta.h"
#include "svn_error.h"
#include "svn_pools.h"
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 12 01:56:45 2004