[patch] remove a few unnecessary #includes

[Rainer Weikusat]

Bernhard Fischer <rep.nop at aon.at> writes:

[...]

> diff -X excl -rduNp busybox.busybox.oorig/archival/bunzip2.c busybox.busybox/archival/bunzip2.c
> --- busybox.busybox.oorig/archival/bunzip2.c	2005-09-02 09:45:28.000000000 +0200
> +++ busybox.busybox/archival/bunzip2.c	2005-09-09 22:13:12.000000000 +0200
> @@ -6,7 +6,6 @@
>   */
>  
>  #include <fcntl.h>
> -#include <getopt.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -35,7 +34,7 @@ int bunzip2_main(int argc, char **argv)
>  		src_fd = STDIN_FILENO;
>  		filename = 0;
>  	}
> -	
> +
>  	/* if called as bzcat force the stdout flag */
>  	if ((opt & BUNZIP2_OPT_STDOUT) || bb_applet_name[2] == 'c')
>  		filename = 0;

That is not a very sensible patch.

> diff -X excl -rduNp busybox.busybox.oorig/archival/cpio.c busybox.busybox/archival/cpio.c
> --- busybox.busybox.oorig/archival/cpio.c	2005-04-01 22:05:36.000000000 +0200
> +++ busybox.busybox/archival/cpio.c	2005-09-09 22:11:37.000000000 +0200
> @@ -24,20 +24,19 @@
>   *
>   */
>  #include <fcntl.h>
> -#include <getopt.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
>  #include "unarchive.h"
>  #include "busybox.h"
>  
> -#define CPIO_OPT_EXTRACT           	0x01
> -#define CPIO_OPT_TEST              	0x02
> -#define CPIO_OPT_UNCONDITIONAL     	0x04
> -#define CPIO_OPT_VERBOSE           	0x08
> -#define CPIO_OPT_FILE              	0x10
> +#define CPIO_OPT_EXTRACT			0x01
> +#define CPIO_OPT_TEST				0x02
> +#define CPIO_OPT_UNCONDITIONAL		0x04
> +#define CPIO_OPT_VERBOSE			0x08
> +#define CPIO_OPT_FILE				0x10
>  #define CPIO_OPT_CREATE_LEADING_DIR	0x20
> -#define CPIO_OPT_PRESERVE_MTIME    	0x40
> +#define CPIO_OPT_PRESERVE_MTIME		0x40

Before you changed the file, the constants where 'properly aligned'.
Now, they aren't anymore. The precise point of this is (except
damaging readability?)?

>  
>  extern int cpio_main(int argc, char **argv)
>  {
> @@ -59,7 +58,7 @@ extern int cpio_main(int argc, char **ar
>  	}
>  
>  	if (opt & CPIO_OPT_TEST) {
> -		/* if both extract and test option are given, ignore extract option */
> +		/* if both extract and test options are given, ignore extract option */

This adds a plural s where none is grammatically needed. This is
called an ellipsis ("if both extract option and test option are
given") and usually considered to be good style.


> @@ -79,7 +78,7 @@ extern int cpio_main(int argc, char **ar
>  			archive_handle->action_header = header_list;
>  		}
>  	}
> -	if (cpio_filename) {	/* CPIO_OPT_FILE */
> +	if (cpio_filename) { /* CPIO_OPT_FILE */

This appears to change what was a HTAB (\x9) into a space (\x20).
The point is?

[...]

> diff -X excl -rduNp busybox.busybox.oorig/coreutils/env.c busybox.busybox/coreutils/env.c
> --- busybox.busybox.oorig/coreutils/env.c	2005-09-05 21:44:17.000000000 +0200
> +++ busybox.busybox/coreutils/env.c	2005-09-10 13:13:58.000000000 +0200
> @@ -47,7 +47,7 @@
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <unistd.h>
> -#include <getopt.h>
> +#include <getopt.h> /* struct option */
>  #include "busybox.h"

... which translates to "ouch -- this wasn't superfloous after all --
better add a note before I delete it again accidentally". Not a
sensible addition (either, comment all include or don't comment
includes). 

> -#include <getopt.h>
> +#include <getopt.h> /* optind */

You should be using unistd.h for that (if only to pose as someone
caring for portability).

> diff -X excl -rduNp busybox.busybox.oorig/debianutils/readlink.c busybox.busybox/debianutils/readlink.c
> --- busybox.busybox.oorig/debianutils/readlink.c	2005-04-01 22:05:37.000000000 +0200
> +++ busybox.busybox/debianutils/readlink.c	2005-09-10 13:23:25.000000000 +0200
> @@ -43,8 +43,6 @@ int readlink_main(int argc, char **argv)
>  	RESERVE_CONFIG_BUFFER(resolved_path, PATH_MAX);
>  #endif
>  
> -	/* no options, no getopt */
> -
>  	if (optind + 1 != argc)
>  		bb_show_usage();

Why delete the comment?

> @@ -58,9 +56,8 @@ int readlink_main(int argc, char **argv)
>  	if (!buf)
>  		return EXIT_FAILURE;
>  	puts(buf);
> -#ifdef CONFIG_FEATURE_CLEAN_UP
> -	free(buf);
> -#endif
> +
> +	if (ENABLE_FEATURE_CLEAN_UP) free(buf);

This doesn't change anything relevant, except that people with older
compilers or compilers for certain architectures will get the
(pointless) free call despite having disabled this "feature" (some
people may event want to *gasp* turn the optimizer off ...).

It is still a bad idea to do this cleanup-stuff, which only ever
makes sense for people who don't run busybox as part of a full-blown
operating system, in each applet, instead of fixing this up *in* *one*
*place* (namely, the exit routine).

And it is still an extraordinary stupid concept to litter source code
with compiler-dependant stuff whose purpose is to make the code worse
except if an optimizer supports certain features. It's not that it is
getting bored if it doesn't have anything to do.

Aber das ist natuerlich Pfeifen im Walde.