On Tue, 21.10.14 15:21, Ken Sedgwick (ksedg...@bonsai.com) wrote: > +#include <errno.h> > +#include <fcntl.h> > +#include <stdio.h> > +#include <string.h> > +#include <sys/param.h> > +#include <unistd.h> > +#include <time.h> > + > +#include "manager.h" > +#include "macro.h" > +#include "util.h" > + > +static const int NUNITS = 3000;
Tss, this is not C++. ;-) Just use #define for this... > + > +static const char *many_path(int unitnum) { > + static char path[PATH_MAX]; > + snprintf(path, PATH_MAX, "%s/%s/many-%06d.service", > + root_dir, "usr/lib/systemd/system", unitnum); > + return path; > +} This is a test, so we don't need to be that strict, but please try to avoid allocating fixed-size strings if possible (unless you are in an inner loop, and you know the limit is short, and it matters because of speed...), use asprintf() instead, to allocate a dynamically sized string. _cleanup_free_ makes it easy to get paths like that cleaned up. Nitpick: we don't break lines that eagerly. > + > +static const char *link_path(int unitnum) { > + static char path[PATH_MAX]; > + snprintf(path, PATH_MAX, "%s/%s/many-%06d.service", > + root_dir, "etc/systemd/system/some.target.wants", unitnum); > + return path; > +} > + > +static const char *another_path(void) { > + static char path[PATH_MAX]; > + snprintf(path, PATH_MAX, "%s/%s/another.service", > + root_dir, "usr/lib/systemd/system"); > + return path; Nitpick: when just concatenating paths we really prefer strjoin() instead of asprintf()... It's waaay faster. (doesn't matter here of course, it's a test, but just wanted to mention that...) > + fprintf(stderr, "testing with %d unit files\n", NUNITS); > + > + t0 = time(NULL); Please don't use time(), use now() instead and store the time in a usec_t. We pretty universally adopted simple timekeeping with now() in systemd, and we really should not start here with time()... > + h = hashmap_new(&string_hash_ops); OOM check please! > + r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h); > + assert_se_cleanup(r >= 0); > + HASHMAP_FOREACH(p, h, i) { > + ++count; > + } Nitpick: single line {} blocks please without the {}... > + fprintf(stderr, "saw %d units\n", count); All fprintf() to stderr should really preferrably be done with log_error() or a related call... > + assert_se_cleanup(count == 3015); > + t1 = time(NULL); > + > + fprintf(stderr, "unit_file_get_list took %ld seconds\n", > + (long) (t1 - t0)); same here... > + > + assert_se_cleanup(t1 - t0 < 10); > +} This looks dangerous should these tests run on some overworked build system host. Otherwise looks great! Thanks, Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel