Skip to content

Lots of small fixes#1835

Merged
zonque merged 13 commits intosystemd:masterfrom
poettering:grabbag-of-stuff
Nov 10, 2015
Merged

Lots of small fixes#1835
zonque merged 13 commits intosystemd:masterfrom
poettering:grabbag-of-stuff

Conversation

@poettering
Copy link
Member

Among other stuff contains fixes for #1829 and #1772.

…ZATION_CONTAINER_OTHER

If we don't know a container manager, we should consider it as "other"
rather than as no container manager at all, to provide a somwhat useful
upgrade path.
The macro is generically useful for putting together search paths, hence
let's make it truly generic, by dropping the implicit ".d" appending it
does, and leave that to the caller. Also rename it from
CONF_DIRS_NULSTR() to CONF_PATHS_NULSTR(), since it's not strictly about
dirs that way, but any kind of file system path.

Also, mark CONF_DIR_SPLIT_USR() as internal macro by renaming it to
_CONF_PATHS_SPLIT_USR() so that the leading underscore indicates that
it's internal.
…cification of default time unit if none is specified

This is useful if we want to parse RLIMIT_RTTIME values where the common
UNIX syntax is without any units but refers to a non-second unit (µs in
this case), but where we want to allow specification of units.
Let's generate a simple error, and that's it. Let's not try to be smart
and record the last word that failed.

Also, let's make sure we don't compare numeric values with 0 by relying
on C's downgrade-to-bool feature, as suggested in CODING_STYLE.
…rner cases

Let's not convert RLIM_INFINITY to "unsigned long long" and then back to
rlim_t, but let's leave it in the right type right-away.

Parse resource limits as 64 bit in all cases, as according to the man
page that's what libc does anyway.

Make sure setting a resource limit to (uint64_t) -1 results in a parsing
error, and isn't implicitly converted to RLIM_INFINITY.
Let's make sure "LimitCPU=30min" can be parsed properly, following the
usual logic how we parse time values. Similar for LimitRTTIME=.

While we are at it, extend a bit on the man page section about resource
limits.

Fixes: systemd#1772
Let's make sure to process all queued log data before exiting, so that
we don't unnecessary lose messages when shutting down.

systemd#1812 (comment)
…ven on x86-32

strtoull() doesn't make it particularly easy to detect passed-in
negative numbers, as it silently converts them to positive ones without
generating any error. Since we are not interested in negative values we
should hence explicitly filter them out by looking at the string
directly and returning ERANGE if we see a leading "-".

Fixes: systemd#1829
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you're doing here... but I find that this looks slightly odd:

  if (errno != 0)
    return -errno;

So if errno is ever negative you'll end up returning a postive value?

Perhaps this would be slightly better?

  if (errno != 0)
    return -abs(errno);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or...

  if (errno != 0) {
    assert(errno > 0);  /* it doesn't make sense otherwise... */
    return -errno;
  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to take a patch that adds an assert() to check for a positive errno. but i figure this can be done independently of this PR...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, after this one is in I'll send you a treewide one, thanks!

@poettering poettering mentioned this pull request Nov 10, 2015
zonque added a commit that referenced this pull request Nov 10, 2015
@zonque zonque merged commit e3c4a68 into systemd:master Nov 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants