On Tue, 22 Apr 2014 17:47:11 +0200
<chru...@suse.cz> wrote:
> > +/*
> > + * Wait for a command with given pid to finish, and assert that
> > the command
> > + * returned success (0).
> > + */
> > +static void
> > +pid_wait(pid_t pid)
> > +{
> > + int status;
> > + char prefix[64];
> > +
> > + snprintf(prefix, sizeof (prefix), "Pid %lu", (unsigned
> > long) pid); +
> > + if (wait(&status) == -1)
> > + err("%s: wait() failed: %s", prefix,
> > strerror(errno));
>
> Use SAFE_WAIT() instead.
>
I couldn't find that function in include directory. Did I search in the
wrong place?
> > +static unsigned long str_to_ulong(
> > + const char *str,
> > + const char *err_prefix,
> > + int base
> > + )
> > +{
> > + char *endptr;
> > + unsigned long val;
> > + val = strtoull(str, &endptr, base);
> > + if (endptr == str)
> > + err("%s: %s: Expected unsigned decimal value",
> > err_prefix, str);
> > + if (*endptr != '\0')
> > + err("%s: %s: Garbage character '%c' found after
> > decimal value",
> > + err_prefix, str, *endptr);
> > +
> > + return val;
> > +}
>
> Use SAFE_STRTOUL() instead.
That would loose a lot of information useful for figuring out what the
error is. For example, when called for translating command line options
to integers, using SAFE_STRTOUL() will only say that "this string is
not an integer" without giving any hint that we are talking about a
command line option, and which option that is being parsed.
> > + pid_t pid;
> > + int status;
> > +
> > + if (!__sync_bool_compare_and_swap(&cleanup_entered, 0, 1,
> > + cleanup_entered)) {
> > + info("Cleanup: Already cleaning, exiting");
> > + return;
> > + }
> > +
> > + info("Cleanup: Terminating children");
> > +
> > + time_is_up = 1;
> > +
> > + do {
> > + pid = wait(&status);
> > + if (pid != -1) {
> > + char cmd[64];
> > + snprintf(cmd, sizeof (cmd), "Pid %lu",
> > + (unsigned long) pid);
> > + assert_exit_status(cmd, status);
>
> I do not understand why you pass "Pid %lu"
> string instead of the pid. It's more messy
> this way.
This is because assert_exit_status() sometimes checks result of
executed commands, and in those cases this function gets a command line
rather than PID.
The option would be to have yet another assert_exit_status() doing the
same thing but accepting a PID instead. Not sure that this is less
messy...
Regards
Mats Liljegren
------------------------------------------------------------------------------
Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
