Package: apt-spy
Severity: minor
Tags: patch
Initialization of defaults can be inlined. I see:
$ grep 'd_.*\[' main.c
main.c:char d_area[] = "All";
main.c:char d_out[] = "/etc/apt/sources.list";
main.c:char d_config[] = "/etc/apt-spy.conf";
main.c:char d_mirror[] = "/var/lib/apt-spy/mirrors.txt";
main.c:char d_file[] = "ls-lR";
main.c:char d_update_url[] =
"http://http.us.debian.org/debian/README.mirrors.txt";;
and then I see:
$ for s in $(grep -o 'd_[^[]*\[' main.c |sed -e 's/\[$//'); do echo -n $s;
grep -h "=.*$s" *.c; done;
d_area area = d_area;
d_out outfile = d_out;
d_config config_file = d_config;
d_mirror mirror_list = d_mirror;
d_file grab_file = d_file;
d_update_url update_url = d_update_url;
I can't see any reason why these can't all be initialized inline with
their declaration, as in
char *area=d_area;
instead of having it set by a runtime conditional. If any of those
static default strings don't have corresponding variables in main(),
then you could still initialize them (in an arbitrary function) by
using #defines, as in
char *foo = FOO_D;
where a new ./includes/defaults.h can specify, in a central location,
all of the FOO_D defaults.
(I believe, however, that each of those 6 static strings have such a
corresponding variable in main).
[ ... ]
I've now written a patch for this. The only complication is d_area,
which requires a minor code change: since it will always be non-NULL,
the check is now country_list!=NULL.
Justin
Only in apt-spy-3.1.jp: apt-spy
diff -ur apt-spy-3.1/benchmark.c apt-spy-3.1.jp/benchmark.c
--- apt-spy-3.1/benchmark.c 2005-07-08 20:48:09.000000000 -0400
+++ apt-spy-3.1.jp/benchmark.c 2005-07-08 21:45:23.000000000 -0400
@@ -10,6 +10,7 @@
#include "include/parse.h"
#include "include/benchmark.h"
#include "include/protocols.h"
+#include "include/global.h"
/*
* It is safer to keep track of the total amount of data read ourselves.
@@ -79,12 +80,12 @@
/* Inefficient sorting algorithm, but small number of entries so it
doesn't matter. */
/* move 'i' to the correct place in the array to place the new entry */
- for (i = 0; i < BESTNUMBER; ++i)
+ for (i = 0; i < bestnumber; ++i)
if (current->stats.speed > best[i].stats.speed)
break;
/* shove everything along one */
- for (j = BESTNUMBER - 2; j >= i; --j)
+ for (j = bestnumber - 2; j >= i; --j)
memcpy(&best[j + 1], &best[j], sizeof(server_t));
/* copy the new entry into the correct place */
Only in apt-spy-3.1.jp: build-stamp
Only in apt-spy-3.1.jp: configure-stamp
diff -ur apt-spy-3.1/file.c apt-spy-3.1.jp/file.c
--- apt-spy-3.1/file.c 2005-07-08 20:48:09.000000000 -0400
+++ apt-spy-3.1.jp/file.c 2005-07-09 14:45:56.000000000 -0400
@@ -52,9 +52,6 @@
{
FILE *fp;
- if (config_file == NULL)
- config_file = d_config;
-
fp = fopen(config_file, "r");
return fp;
@@ -64,9 +61,6 @@
{
FILE *fp;
- if (mirror_list == NULL)
- mirror_list = d_mirror;
-
if (are_updating == 1)
fp = fopen(mirror_list, "w");
else
diff -ur apt-spy-3.1/include/global.h apt-spy-3.1.jp/include/global.h
--- apt-spy-3.1/include/global.h 2003-12-18 10:12:24.000000000 -0500
+++ apt-spy-3.1.jp/include/global.h 2005-07-09 17:47:15.000000000 -0400
@@ -3,9 +3,26 @@
#ifndef __GLOBAL_H
#define __GLOBAL_H
-extern char apt_spy_v[];
-extern char d_out[];
-extern char d_config[];
-extern char d_mirror[];
+/* Defaults values */
+
+/* Our version number. */
+#define apt_spy_v "v3.1"
+
+/* The default area */
+#define D_AREA "All"
+
+/* Default file locations */
+#define D_OUT "/etc/apt/sources.list"
+#define D_CONFIG "/etc/apt-spy.conf"
+#define D_MIRROR "/var/lib/apt-spy/mirrors.txt"
+
+/* Default file to grab when benchmarking */
+#define D_FILE "ls-lR"
+
+/* Default update URL */
+#define D_UPDATE_URL
"http://http.us.debian.org/debian/README.mirrors.txt";
+
+#define BESTNUMBER 5
+extern int bestnumber;
#endif
diff -ur apt-spy-3.1/include/parse.h apt-spy-3.1.jp/include/parse.h
--- apt-spy-3.1/include/parse.h 2005-07-08 20:48:09.000000000 -0400
+++ apt-spy-3.1.jp/include/parse.h 2005-07-08 21:06:35.000000000 -0400
@@ -6,9 +6,6 @@
#define FTP 0
#define HTTP 1
-/* hack */
-extern int BESTNUMBER;
-
struct stats_struct {
int protocol;
double speed;
diff -ur apt-spy-3.1/main.c apt-spy-3.1.jp/main.c
--- apt-spy-3.1/main.c 2005-07-08 20:48:09.000000000 -0400
+++ apt-spy-3.1.jp/main.c 2005-07-09 17:48:19.000000000 -0400
@@ -16,27 +16,9 @@
#include "include/file.h"
#include "include/parse.h"
#include "include/benchmark.h"
+#include "include/global.h"
-/* Our version number. */
-const char apt_spy_v[] = "v3.1";
-
-/* Defaults */
-
-/* The default area */
-char d_area[] = "All";
-
-/* Default file locations */
-char d_out[] = "/etc/apt/sources.list";
-char d_config[] = "/etc/apt-spy.conf";
-char d_mirror[] = "/var/lib/apt-spy/mirrors.txt";
-
-/* Default file to grab when benchmarking */
-char d_file[] = "ls-lR";
-
-/* Default update URL */
-char d_update_url[] = "http://http.us.debian.org/debian/README.mirrors.txt";;
-
-int BESTNUMBER = 5;
+int bestnumber=BESTNUMBER;
int main(int argc, char *argv[])
{
@@ -44,15 +26,15 @@
char *cur_entry; /* Entry we are benchmarking */
char *distrib = NULL; /* distrubtion to use. */
- char *mirror_list = NULL; /* mirror list file */
- char *config_file = NULL; /* configuraion file */
+ char *mirror_list = D_MIRROR; /* mirror list file */
+ char *config_file = D_CONFIG; /* configuraion file */
char *proxy = NULL; /* Proxy server to use */
char *infile = NULL; /* optional infile */
- char *outfile = NULL; /* outfile name */
+ char *outfile = D_OUT; /* outfile name */
char *topfile = NULL;
- char *area = NULL; /* Area to test */
- char *grab_file = NULL; /* File to grab */
- char *update_url = NULL; /* URL to use for updating */
+ char *area = strdup(D_AREA); /* Area to test; modified in
build_area_file*/
+ char *grab_file = D_FILE; /* File to grab */
+ char *update_url = D_UPDATE_URL;/* URL to use for updating */
char *country_list = NULL; /* List of countries to b/m */
FILE *infile_p, *outfile_p; /* input/output file pointers */
FILE *config_p; /* config file pointer */
@@ -126,7 +108,7 @@
break;
/* Number of servers to write in "top" server list */
case 'n':
- BESTNUMBER = atoi(optarg);
+ bestnumber = atoi(optarg);
break;
case 'v':
version();
@@ -141,10 +123,10 @@
argv += optind;
/* Simple check for stupidity */
- if ((test_number >= 0) && (BESTNUMBER > test_number))
- BESTNUMBER = test_number;
+ if ((test_number >= 0) && (bestnumber > test_number))
+ bestnumber = test_number;
- best = malloc(sizeof(server_t) * (BESTNUMBER + 1));
+ best = malloc(sizeof(server_t) * (bestnumber + 1));
if (best == NULL) {
perror("malloc");
@@ -156,16 +138,8 @@
usage();
/* Check for silly combination of country and area arguments */
- if ((area != NULL) && (country_list != NULL))
- usage();
-
- /* Setup default area argument */
- if ((area == NULL) && (country_list == NULL))
- area = d_area;
-
- /* Setup default file argument if none given */
- if (grab_file == NULL)
- grab_file = d_file;
+ if (strcmp(area, D_AREA) && (country_list != NULL))
+ usage();
/* Open mirror file. We pass argc so it can tell if we're updating
or not */
@@ -182,10 +156,6 @@
if (strcmp(argv[0], "update") != 0)
usage();
- /* If necessary, set update_url to default */
- if (update_url == NULL)
- update_url = d_update_url;
-
if (update(mirror_p, update_url, proxy) != 0) {
fprintf(stderr, "Update failed. Exiting.\n");
exit(1);
@@ -197,7 +167,6 @@
/* argc should be 0. If not, there's something wrong. */
if (argc != 0)
usage();
-
/* We open the infile. Either a temporary file, or a user-specified
one. */
@@ -208,10 +177,6 @@
exit(1);
}
- /* Set up default if user hasn't specified an outfile */
- if (outfile == NULL)
- outfile = d_out;
-
/* Check output file for accessibility */
if (check_write_access(outfile) == 1) {
fprintf(stderr, "Could not open outfile. Exiting.\n");
@@ -236,18 +201,19 @@
/* Fill temporary file with useful stuff if it's not user-specified. */
if (infile == NULL) {
- if (area != NULL) {
- if (build_area_file(config_p, infile_p, mirror_p, area)
!= 0) {
- fprintf(stderr,
- "Error building area file. Exiting.\n");
- exit(1);
- }
- } else {
+ if (country_list) {
if (build_country_file(config_p, infile_p, mirror_p,
country_list) != 0) {
fprintf(stderr,
"Error building country file. Exiting.\n");
exit(1);
}
+
+ } else {
+ if (build_area_file(config_p, infile_p, mirror_p, area)
!= 0) {
+ fprintf(stderr,
+ "Error building area file. Exiting.\n");
+ exit(1);
+ }
}
}
@@ -256,7 +222,7 @@
rewind(infile_p);
/* Zero the "best" structure */
- for (c = 0; c < BESTNUMBER; c++)
+ for (c = 0; c < bestnumber; c++)
memset(&best[c], 0, sizeof(server_t));
/* This is the main loop. It'll exit when we've exhausted the URL
diff -ur apt-spy-3.1/parse.c apt-spy-3.1.jp/parse.c
--- apt-spy-3.1/parse.c 2005-07-08 20:48:09.000000000 -0400
+++ apt-spy-3.1.jp/parse.c 2005-07-08 21:45:26.000000000 -0400
@@ -359,7 +359,7 @@
int i = 0;
char *line;
- while (i < BESTNUMBER) {
+ while (i < bestnumber) {
/* Make sure we're at the beginning */
rewind(infile_p);
Only in apt-spy-3.1.jp: test
