Re: [systemd-devel] [PATCH 2/2] sysv-generator: remove NULL pointer dereference
Actually you missed that free_sysvstub_hashmap does not tolerate NULL pointers.
On Sun, Apr 26, 2015 at 11:21 AM, Shawn Landden sh...@churchofgit.com wrote:
On Sun, Apr 26, 2015 at 11:15 AM, Thomas H.P. Andersen pho...@gmail.com
wrote:
Hi Shawn,
I fixed this a few hours ago. I also updated the status in coverity.
Is there something else I can do to avoid duplicated work?
I wasn't checking coverity, just reading the emails, so the duplicated
work in on my end.
On Sun, Apr 26, 2015 at 7:58 PM, Shawn Landden sh...@churchofgit.com wrote:
(coverity)
---
src/sysv-generator/sysv-generator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/sysv-generator/sysv-generator.c
b/src/sysv-generator/sysv-generator.c
index 5ecd750..714ce8f 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -922,7 +922,7 @@ finish:
int main(int argc, char *argv[]) {
int r, q;
_cleanup_lookup_paths_free_ LookupPaths lp = {};
-_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services;
+_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services = NULL;
SysvStub *service;
Iterator j;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] sysv-generator: remove NULL pointer dereference
static void free_sysvstub_hashmapp(Hashmap **h) {
while ((stub = hashmap_steal_first(*h)))
_cleanup_ sends a pointer to the pointer. and then this dereferences
that, which is kinda confusing, but yeah the code is correct, it would
be clearer with DEFINE_TRIVIAL_CLEANUP_FUNC()
On Sun, Apr 26, 2015 at 12:04 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
On Sun, Apr 26, 2015 at 8:31 PM, Thomas H.P. Andersen pho...@gmail.com
wrote:
On Sun, Apr 26, 2015 at 8:23 PM, Shawn Landden sh...@churchofgit.com wrote:
Actually you missed that free_sysvstub_hashmap does not tolerate NULL
pointers.
Indeed. I will commit that.
Wait. free_sysvstub_hashmapp does tolerate NULL pointers.
hashmap_steal_first will return NULL if the hashmap is NULL. And
hashmap_free is fine with NULL too. Your patch makes it more obvious
that free_sysvstub_hashmapp does tolerate NULL but destructors should
tolerate NULL as per the coding style. So I guess it should just be
assumed? I will leave it up to the others to decide what the best
style is here.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] path-util: fix fd_is_mount_point
(coverity)
---
src/shared/path-util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index 925bb28..95bfafc 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -627,7 +627,7 @@ fallback_fstat:
a.st_ino == b.st_ino)
return 1;
-return check_st_dev (a.st_dev != b.st_dev);
+return (check_st_dev (a.st_dev != b.st_dev));
}
int path_is_mount_point(const char *t, bool allow_symlink) {
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] sysv-generator: remove NULL pointer dereference
(coverity)
---
src/sysv-generator/sysv-generator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/sysv-generator/sysv-generator.c
b/src/sysv-generator/sysv-generator.c
index 5ecd750..714ce8f 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -922,7 +922,7 @@ finish:
int main(int argc, char *argv[]) {
int r, q;
_cleanup_lookup_paths_free_ LookupPaths lp = {};
-_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services;
+_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services = NULL;
SysvStub *service;
Iterator j;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] sysv-generator: remove NULL pointer dereference
On Sun, Apr 26, 2015 at 11:15 AM, Thomas H.P. Andersen pho...@gmail.com wrote:
Hi Shawn,
I fixed this a few hours ago. I also updated the status in coverity.
Is there something else I can do to avoid duplicated work?
I wasn't checking coverity, just reading the emails, so the duplicated
work in on my end.
On Sun, Apr 26, 2015 at 7:58 PM, Shawn Landden sh...@churchofgit.com wrote:
(coverity)
---
src/sysv-generator/sysv-generator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/sysv-generator/sysv-generator.c
b/src/sysv-generator/sysv-generator.c
index 5ecd750..714ce8f 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -922,7 +922,7 @@ finish:
int main(int argc, char *argv[]) {
int r, q;
_cleanup_lookup_paths_free_ LookupPaths lp = {};
-_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services;
+_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services = NULL;
SysvStub *service;
Iterator j;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] sysv-generator: do not dereference uninitilized data or NULL pointer
(coverity)
v2
---
src/sysv-generator/sysv-generator.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/sysv-generator/sysv-generator.c
b/src/sysv-generator/sysv-generator.c
index 5ecd750..f56d727 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -99,6 +99,9 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(SysvStub*, free_sysvstub);
static void free_sysvstub_hashmapp(Hashmap **h) {
SysvStub *stub;
+if (h == NULL)
+return;
+
while ((stub = hashmap_steal_first(*h)))
free_sysvstub(stub);
@@ -922,7 +925,7 @@ finish:
int main(int argc, char *argv[]) {
int r, q;
_cleanup_lookup_paths_free_ LookupPaths lp = {};
-_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services;
+_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services = NULL;
SysvStub *service;
Iterator j;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 5/6] network: fix strict aliasing issue
On Sun, Apr 12, 2015 at 12:43 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Wed, 11.03.15 08:13, Shawn Landden (sh...@churchofgit.com) wrote:
We shouldn't assume 64-bit arch with the way we do math either.
(although I will submit a patch to glibc to add a uint64_t union
alias)
Hmm? uint64_t works fine on 32bit too. The compiler can do the
necessary emulation on its own... I don't see the need to change
anything here.
Its also an unaligned access. in_addr.in6 is only 32-bit aligned. (in
addition to being a strict aliasing violation.)
diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 0be6165..4b7f451 100644
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -605,12 +605,12 @@ bool address_equal(Address *a1, Address *a2) {
}
case AF_INET6: {
-uint64_t *b1, *b2;
+uint32_t *b1, *b2;
-b1 = (uint64_t*)a1-in_addr.in6;
-b2 = (uint64_t*)a2-in_addr.in6;
+b1 = a1-in_addr.in6.s6_addr32[0];
+b2 = a2-in_addr.in6.s6_addr32[0];
-return (((b1[0] ^ b2[0]) | (b1[1] ^ b2[1])) == 0UL);
+return (((b1[0] ^ b2[0]) | (b1[1] ^ b2[1]) | (b1[2] ^
b2[2]) | (b1[3] ^ b2[3])) == 0);
}
default:
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] network: allow domain names up to 255 characters
On Sun, Apr 12, 2015 at 6:35 AM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 10.04.15 13:03, Nick Owens (misch...@offblast.org) wrote: From: mischief misch...@offblast.org The maximum domain name size is larger than the maximum host name size. The smaller limit causes valid domains provided by DHCP or .network files to be silently ignored. Hmm? Can you give an example? What is a domain name according to your definition? And what a hostname? So far, a hostname in my definition was either a single label, or an fqdn, and a domain name the part of the fqdn with the first label removed... With such a definition I am not sure I understand the patch, hence please explain, and give a valid example of where this turns out to be an issue? ./x86_64-linux-gnu/bits/posix1_lim.h:#define _POSIX_HOST_NAME_MAX 255 ./x86_64-linux-gnu/bits/local_lim.h:#define HOST_NAME_MAX 64 ./x86_64-linux-musl/limits.h:#define HOST_NAME_MAX 255 Perhaps we need to use _POSIX_HOST_NAME_MAX ?, or redefine HOST_NAME_MAX to 255? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] network: allow domain names up to 255 characters
On Sun, Apr 12, 2015 at 6:35 AM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 10.04.15 13:03, Nick Owens (misch...@offblast.org) wrote: From: mischief misch...@offblast.org The maximum domain name size is larger than the maximum host name size. The smaller limit causes valid domains provided by DHCP or .network files to be silently ignored. Hmm? Can you give an example? What is a domain name according to your definition? And what a hostname? So far, a hostname in my definition was either a single label, or an fqdn, and a domain name the part of the fqdn with the first label removed... With such a definition I am not sure I understand the patch, hence please explain, and give a valid example of where this turns out to be an issue? ./x86_64-linux-gnu/bits/posix1_lim.h:#define _POSIX_HOST_NAME_MAX 255 ./x86_64-linux-gnu/bits/local_lim.h:#define HOST_NAME_MAX 64 ./x86_64-linux-musl/limits.h:#define HOST_NAME_MAX 255 Perhaps we need to use _POSIX_HOST_NAME_MAX ?, or redefine HOST_NAME_MAX to 255? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd-network src/udev
On Fri, Apr 10, 2015 at 07:54:33PM +0200, Ronny Chevalier wrote:
On Fri, Apr 10, 2015 at 7:05 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Sat, 14.03.15 06:54, Ronny Chevalier (rcheval...@kemper.freedesktop.org)
wrote:
commit 6ec8e7c763b7dfa82e25e31f6938122748d1608f
Author: Shawn Landden sh...@churchofgit.com
Date: Tue Mar 10 20:45:15 2015 -0700
sd-dhcp-client: fix strict aliasing issue
diff --git a/src/libsystemd-network/sd-dhcp-client.c
b/src/libsystemd-network/sd-dhcp-client.c
index 4224e01..a477ccc 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -1469,7 +1469,7 @@ static int
client_receive_message_udp(sd_event_source *s, int fd,
_cleanup_free_ DHCPMessage *message = NULL;
int buflen = 0, len, r;
const struct ether_addr zero_mac = { { 0, 0, 0, 0, 0, 0 } };
-const struct ether_addr *expected_chaddr = NULL;
+bool expect_chaddr;
uint8_t expected_hlen = 0;
assert(s);
@@ -1514,11 +1514,11 @@ static int
client_receive_message_udp(sd_event_source *s, int fd,
if (client-arp_type == ARPHRD_ETHER) {
expected_hlen = ETH_ALEN;
-expected_chaddr = (const struct ether_addr *)
client-mac_addr;
+expect_chaddr = true;
} else {
/* Non-ethernet links expect zero chaddr */
expected_hlen = 0;
- expected_chaddr = zero_mac;
+ expect_chaddr = false;
}
if (message-hlen != expected_hlen) {
@@ -1526,7 +1526,10 @@ static int
client_receive_message_udp(sd_event_source *s, int fd,
return 0;
}
-if (memcmp(message-chaddr[0], expected_chaddr, ETH_ALEN)) {
+if (memcmp(message-chaddr[0], expect_chaddr ?
+ (void *)client-mac_addr :
+ (void *)zero_mac,
+ETH_ALEN)) {
log_dhcp_client(client, received chaddr does not match
expected: ignoring);
return 0;
Ahum, what is this about? Please elaborate what kind of struct aliasing
this fixes?
I think Shawn was trying to avoid to cast mac_addr which is (uint8_t
*) to a (const struct ether_addr *) which breaks the strict aliasing
rule. Or I misunderstood the patch? Shawn?
-expected_chaddr = (const struct ether_addr *)
client-mac_addr;
The compiler knows very well what we are taking the pointer of... and
memcmp() doesn't really care anyway it takes (void*) pointers anyway...
The issue is not with memcmp but with the cast from a (uint8_t *) to a
(const struct ether_addr *) which breaks the strict aliasing rule.
Given that we don't do any operations on it (besides memcmp which doesn't
matter)
it doesn't actually violate, but it does generate an annoying warning.
If I'm not mistaken Shawn is trying to remove all the strict aliasing
violations reported by gcc, in order to add the warning later on.
Ronny
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] sd_event_run
On Sat, Apr 11, 2015 at 4:52 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Fri, Apr 10, 2015 at 06:14:40PM +0200, Lennart Poettering wrote: On Sat, 14.03.15 12:19, Tom Gundersen (t...@jklm.no) wrote: 1. shouldn't SD_EVENT_PASSIVE become SD_EVENT_INITIAL? passive seems strange in this context. How about SD_EVENT_STALE? Stale things are distasteful unless they are freshened up. I found it weird to name this INITIAL (or INIT or something like that) since we would return to it every single iteration... For me init kinda implies it's something fresh, unused, and hence not really something we routinely come back to... In a sense it is fresh and unused, because the state after doing an interation of the loop is exactly the same as if the loop never run. (E.g. I don't think it would be wrong to say I cleaned the couch and restored it to its initial state, even though it obviously has been used.) Passive was confusing, so initial, even if imperfect, seems an improvement. We really want to say the state in which you started, without implying whether we have been there the whole time or not. I don't know if there's a better word. Similarly, SD_EVENT_ARMED seems more self-explanatory than PREPARED. (I don't like PREPARED because it is not obvious whether sources are prepared to wait on, or events are prepared to be reaped.) I called this prepared since it what's _prepare() is supposed to get into... But ARMED is fine too... We really should get some native speakers to help out with this :) Maybe sd_event_prepare should be renamed to sd_event_arm? Armed is much better because it fits the analogy that events *fire*, after which they are pending. Without reading the header I would think _prepare() is the same as _new(). Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] sd_event_run
On Sat, Apr 11, 2015 at 6:01 PM, Shawn Landden shawnland...@gmail.com wrote: On Sat, Apr 11, 2015 at 4:52 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Fri, Apr 10, 2015 at 06:14:40PM +0200, Lennart Poettering wrote: On Sat, 14.03.15 12:19, Tom Gundersen (t...@jklm.no) wrote: 1. shouldn't SD_EVENT_PASSIVE become SD_EVENT_INITIAL? passive seems strange in this context. How about SD_EVENT_STALE? Stale things are distasteful unless they are freshened up. Nah, Prepared is good. But this SD_EVENT_PASSIVE and SD_EVENT_RUNNING is incompatible with making this multithreaded. Instead there could be a function (int sd_event_references(sd_event *e)) that returns e-n_ref. I found it weird to name this INITIAL (or INIT or something like that) since we would return to it every single iteration... For me init kinda implies it's something fresh, unused, and hence not really something we routinely come back to... In a sense it is fresh and unused, because the state after doing an interation of the loop is exactly the same as if the loop never run. (E.g. I don't think it would be wrong to say I cleaned the couch and restored it to its initial state, even though it obviously has been used.) Passive was confusing, so initial, even if imperfect, seems an improvement. We really want to say the state in which you started, without implying whether we have been there the whole time or not. I don't know if there's a better word. Similarly, SD_EVENT_ARMED seems more self-explanatory than PREPARED. (I don't like PREPARED because it is not obvious whether sources are prepared to wait on, or events are prepared to be reaped.) I called this prepared since it what's _prepare() is supposed to get into... But ARMED is fine too... We really should get some native speakers to help out with this :) Maybe sd_event_prepare should be renamed to sd_event_arm? Armed is much better because it fits the analogy that events *fire*, after which they are pending. Without reading the header I would think _prepare() is the same as _new(). Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] sd-dhcp-client: shutup gcc aliasing warning
we only access this as void* so there is no violation
---
src/libsystemd-network/sd-dhcp-client.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libsystemd-network/sd-dhcp-client.c
b/src/libsystemd-network/sd-dhcp-client.c
index c44392e..bf57d4b 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -1470,7 +1470,7 @@ static int client_receive_message_udp(sd_event_source *s,
int fd,
_cleanup_free_ DHCPMessage *message = NULL;
int buflen = 0, len, r;
const struct ether_addr zero_mac = { { 0, 0, 0, 0, 0, 0 } };
-const struct ether_addr *expected_chaddr = NULL;
+const void *expected_chaddr = NULL;
uint8_t expected_hlen = 0;
assert(s);
@@ -1515,7 +1515,7 @@ static int client_receive_message_udp(sd_event_source *s,
int fd,
if (client-arp_type == ARPHRD_ETHER) {
expected_hlen = ETH_ALEN;
-expected_chaddr = (const struct ether_addr *)
client-mac_addr;
+expected_chaddr = client-mac_addr;
} else {
/* Non-ethernet links expect zero chaddr */
expected_hlen = 0;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] bootchart: fix check for no fd
found with coverty report
---
src/bootchart/store.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index f19427e..f159cba 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -476,7 +476,7 @@ catch_rename:
/* re-fetch name */
/* get name, start time */
-if (!ps-sched) {
+if (ps-sched 0) {
sprintf(filename, %d/sched, pid);
ps-sched = openat(procfd, filename,
O_RDONLY|O_CLOEXEC);
if (ps-sched 0)
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set
On Tue, Mar 31, 2015 at 8:38 AM, Djalal Harouni tix...@opendz.org wrote: On Mon, Mar 30, 2015 at 07:32:35PM -0700, Shawn Landden wrote: On Mon, Mar 30, 2015 at 5:04 PM, Djalal Harouni tix...@opendz.org wrote: On Fri, Mar 27, 2015 at 09:51:26AM -0700, Shawn Landden wrote: On Fri, Mar 27, 2015 at 8:16 AM, Tom Gundersen t...@jklm.no wrote: [...] * Current expression may modify/interact with a global state which may cause a fatal error, and if the caller wants to know if that failed, then abort(), your patch just introduced a regression, without the explicit abort(), all callers now have to call abort(). Yeah, this is the point I think. I still think the patch makes sense though, it really don't see why there should be a distinction between assert_se() and assert() when it comes to logging and aborting. If assert_se() fails it indicates we may have messed up the global state, and if assert() fails it indicates that the global state may be messed up (by someone else). Either way the global state is potentially messed up and not logging/aborting seems as (un)safe in both situations. Another way of seeing it, intuitively I don't see why we should distinguish between assert_se(foo() = 0); and r = foo(); assert(r = 0); Yes. The use case is that embedded people don't want all the strings that the logging adds to their binaries when these are ASSERTS, they are only expected to catch programming errors. It is not to make it faster, I think that is negligible. Hmm embedded cases are real, I had to deal with some in the past. But not sure here since I didn't see any numbers before/after stripping, but perhaps you can start by updating the callers and their semantics if you think that the code there is robust and it's worth it... ? How about we leave the abort() and just drop the logging with NDEBUG set? Not sure if this is the best solution, you will just hide the path of code where we failed! it will not be easy to debug later. As said, the callers except to have a log error and abort, if you remove one of them you are altering callers, IMO they do not handle errors and they do not log errors, they rely on assert_se(). As you know in systemd we do log errors... I think the performance impact is negligible, I just want to drop all the strings. Perhaps we could replace the logging with backtrace() in this case? Don't know are you sure that if you add a backrack() routine there it will be better than the current assert_se(), actually without real numbers it is hard to say... IMO we should not touch this macro, the fix should be in the callers, so you have to make sure that callers are robust and can handle errors correctly (log them too perhaps)... The point is that assert() and assert_se() should only be used for programming errors, this is covered in CODING_STYLE. -- Djalal Harouni http://opendz.org ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Shawn Landden ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Drop systemd-ui
On Tue, Mar 31, 2015 at 7:19 AM, Dimitri John Ledkov dimitri.j.led...@intel.com wrote: On 31 March 2015 at 15:08, Shawn Landden shawnland...@gmail.com wrote: On Tue, Mar 31, 2015 at 12:31 AM, Jóhann B. Guðmundsson johan...@gmail.com wrote: On 03/31/2015 02:30 AM, Shawn Landden wrote: On Mon, Mar 30, 2015 at 4:02 PM, Jóhann B. Guðmundsson johan...@gmail.com wrote: On 03/30/2015 10:32 PM, Shawn Landden wrote: On Mon, Mar 30, 2015 at 1:35 PM, Jóhann B. Guðmundsson johan...@gmail.com wrote: Heyja Should this not be dropped and *DE write,integrate/implement an graphical frontend to systemd for themselves? It's not like this is receiving the love it needs, hence I'm pretty sure nobody is using this. Parts of systemd arn't getting the love they need either, such as systemctl show. For systemd ui the *DE communities are better suited to implement, integrate and maintain an ui on top of systemd for their *DE. https://wiki.gnome.org/Apps/Logs I see the dbus type problems you are probably getting. Isn't this actually a bug in systemd as we are still exporting as systemd1? If we are incompatible shouldn't we be systemd2? I'm not following, I'm not getting any bugs. (systemadm:167530): GLib-GIO-WARNING **: Dropping signal JobNew of type (uos) since the type from the expected interface is (uo) (systemadm:167530): GLib-GIO-WARNING **: Dropping signal JobRemoved of type (uoss) since the type from the expected interface is (uos) Are the systemd-ui systemd out of sync on this machine? Built against systemd 25, run against systemd 218, I will check out what is going on Sure the DBus signature of that method was changed in 2012... but reverting that commit _now_ will induce yet another API break. Note that imho the current JobNew/JobRemoved signals are imho incomplete - See: http://lists.freedesktop.org/archives/systemd-devel/2015-January/026928.html But it is again too late to break the DBus api again. See comments on that thread. How about adding a JobNew2, JobRemoved2 ? This should have happened last time instead of a break. commit 06dab8e18aebf822392c7ca66c5bf3c1200fdec8 Author: Lennart Poettering lenn...@poettering.net Date: Thu May 3 22:53:25 2012 +0200 dbus: include unit name in JobNew/JobRemoved signals This breaks D-Bus interface slightly, but since the D-Bus API isn't covered by the interface stability promise this should be OK. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 6655f29..b5b5113 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -198,10 +198,12 @@ signal name=\JobNew\\n \ arg name=\id\ type=\u\/\n\ arg name=\job\ type=\o\/\n \ + arg name=\unit\ type=\s\/\n \ /signal\n \ signal name=\JobRemoved\\n \ arg name=\id\ type=\u\/\n\ arg name=\job\ type=\o\/\n \ + arg name=\unit\ type=\s\/\n \ arg name=\result\ type=\s\/\n\ /signal \ signal name=\StartupFinished\\n \ As far as I know Kay and Lennart wrote that ui as an proof of concept 4 years ago, to get people started on writing their own ui not as something that should be carried or maintained in longterm in systemd. Since then cockpit has emerged as an frontend to systemd as well as kcmsystemd for KDE and atleast a journal log parser in Gnome. What I'm proposing is that we dropped that proof of concept since it's not being maintained, there exist better alternatives thus it's intended purpose has been fulfilled already. JBG -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Regards, Dimitri. https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Drop systemd-ui
On Tue, Mar 31, 2015 at 10:35 AM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 30.03.15 19:30, Shawn Landden (shawnland...@gmail.com) wrote: What do you feel is missing from systemctl show? It is only suppose to show fields that have been changed by humans (even the developer) not systemd defaults. Hmm? It supresses empty fields by default, unless you specify --all in which cases it shows everything. It really is supposed to show you everything that is in effect, not just the stuff humans configured. If you want that, use systemcl cat. I mean, systemctl show exists precisely to have a look at the effect of implicit dependencies and such, which are otherwise difficult to figure out. I should have been clearer about the language. Yes all this stuff it should show, but: LimitCPU=18446744073709551615 LimitFSIZE=18446744073709551615 LimitDATA=18446744073709551615 LimitSTACK=18446744073709551615 LimitCORE=18446744073709551615 LimitRSS=18446744073709551615 LimitNOFILE=4096 LimitAS=18446744073709551615 LimitNPROC=11881 LimitMEMLOCK=65536 LimitLOCKS=18446744073709551615 LimitSIGPENDING=11881 LimitMSGQUEUE=819200 LimitNICE=0 LimitRTPRIO=0 LimitRTTIME=18446744073709551615 CPUShares=18446744073709551615 StartupCPUShares=18446744073709551615 CPUQuotaPerSecUSec=infinity BlockIOAccounting=no BlockIOWeight=18446744073709551615 StartupBlockIOWeight=18446744073709551615 MemoryAccounting=no MemoryLimit=18446744073709551615 OOMScoreAdjust=0 Nice=0 IOScheduling=0 CPUSchedulingPolicy=0 CPUSchedulingPriority=0 TimerSlackNSec=5 These are all kernel defaults. These ARE empty, and a few of those are even 0. Lennart -- Lennart Poettering, Red Hat -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Drop systemd-ui
On Tue, Mar 31, 2015 at 12:31 AM, Jóhann B. Guðmundsson johan...@gmail.com wrote: On 03/31/2015 02:30 AM, Shawn Landden wrote: On Mon, Mar 30, 2015 at 4:02 PM, Jóhann B. Guðmundsson johan...@gmail.com wrote: On 03/30/2015 10:32 PM, Shawn Landden wrote: On Mon, Mar 30, 2015 at 1:35 PM, Jóhann B. Guðmundsson johan...@gmail.com wrote: Heyja Should this not be dropped and *DE write,integrate/implement an graphical frontend to systemd for themselves? It's not like this is receiving the love it needs, hence I'm pretty sure nobody is using this. Parts of systemd arn't getting the love they need either, such as systemctl show. For systemd ui the *DE communities are better suited to implement, integrate and maintain an ui on top of systemd for their *DE. https://wiki.gnome.org/Apps/Logs I see the dbus type problems you are probably getting. Isn't this actually a bug in systemd as we are still exporting as systemd1? If we are incompatible shouldn't we be systemd2? I'm not following, I'm not getting any bugs. (systemadm:167530): GLib-GIO-WARNING **: Dropping signal JobNew of type (uos) since the type from the expected interface is (uo) (systemadm:167530): GLib-GIO-WARNING **: Dropping signal JobRemoved of type (uoss) since the type from the expected interface is (uos) commit 06dab8e18aebf822392c7ca66c5bf3c1200fdec8 Author: Lennart Poettering lenn...@poettering.net Date: Thu May 3 22:53:25 2012 +0200 dbus: include unit name in JobNew/JobRemoved signals This breaks D-Bus interface slightly, but since the D-Bus API isn't covered by the interface stability promise this should be OK. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 6655f29..b5b5113 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -198,10 +198,12 @@ signal name=\JobNew\\n \ arg name=\id\ type=\u\/\n\ arg name=\job\ type=\o\/\n \ + arg name=\unit\ type=\s\/\n \ /signal\n \ signal name=\JobRemoved\\n \ arg name=\id\ type=\u\/\n\ arg name=\job\ type=\o\/\n \ + arg name=\unit\ type=\s\/\n \ arg name=\result\ type=\s\/\n\ /signal \ signal name=\StartupFinished\\n \ As far as I know Kay and Lennart wrote that ui as an proof of concept 4 years ago, to get people started on writing their own ui not as something that should be carried or maintained in longterm in systemd. Since then cockpit has emerged as an frontend to systemd as well as kcmsystemd for KDE and atleast a journal log parser in Gnome. What I'm proposing is that we dropped that proof of concept since it's not being maintained, there exist better alternatives thus it's intended purpose has been fulfilled already. JBG -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Drop systemd-ui
On Mon, Mar 30, 2015 at 1:35 PM, Jóhann B. Guðmundsson johan...@gmail.com wrote: Heyja Should this not be dropped and *DE write,integrate/implement an graphical frontend to systemd for themselves? It's not like this is receiving the love it needs, hence I'm pretty sure nobody is using this. Parts of systemd arn't getting the love they need either, such as systemctl show. JBG ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set
On Mon, Mar 30, 2015 at 5:04 PM, Djalal Harouni tix...@opendz.org wrote: On Fri, Mar 27, 2015 at 09:51:26AM -0700, Shawn Landden wrote: On Fri, Mar 27, 2015 at 8:16 AM, Tom Gundersen t...@jklm.no wrote: [...] * Current expression may modify/interact with a global state which may cause a fatal error, and if the caller wants to know if that failed, then abort(), your patch just introduced a regression, without the explicit abort(), all callers now have to call abort(). Yeah, this is the point I think. I still think the patch makes sense though, it really don't see why there should be a distinction between assert_se() and assert() when it comes to logging and aborting. If assert_se() fails it indicates we may have messed up the global state, and if assert() fails it indicates that the global state may be messed up (by someone else). Either way the global state is potentially messed up and not logging/aborting seems as (un)safe in both situations. Another way of seeing it, intuitively I don't see why we should distinguish between assert_se(foo() = 0); and r = foo(); assert(r = 0); Yes. The use case is that embedded people don't want all the strings that the logging adds to their binaries when these are ASSERTS, they are only expected to catch programming errors. It is not to make it faster, I think that is negligible. Hmm embedded cases are real, I had to deal with some in the past. But not sure here since I didn't see any numbers before/after stripping, but perhaps you can start by updating the callers and their semantics if you think that the code there is robust and it's worth it... ? How about we leave the abort() and just drop the logging with NDEBUG set? I think the performance impact is negligible, I just want to drop all the strings. Perhaps we could replace the logging with backtrace() in this case? Thanks! -- Djalal Harouni http://opendz.org ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Shawn Landden ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Drop systemd-ui
On Mon, Mar 30, 2015 at 4:02 PM, Jóhann B. Guðmundsson johan...@gmail.com wrote: On 03/30/2015 10:32 PM, Shawn Landden wrote: On Mon, Mar 30, 2015 at 1:35 PM, Jóhann B. Guðmundsson johan...@gmail.com wrote: Heyja Should this not be dropped and *DE write,integrate/implement an graphical frontend to systemd for themselves? It's not like this is receiving the love it needs, hence I'm pretty sure nobody is using this. Parts of systemd arn't getting the love they need either, such as systemctl show. For systemd ui the *DE communities are better suited to implement, integrate and maintain an ui on top of systemd for their *DE. https://wiki.gnome.org/Apps/Logs I see the dbus type problems you are probably getting. Isn't this actually a bug in systemd as we are still exporting as systemd1? If we are incompatible shouldn't we be systemd2? What do you feel is missing from systemctl show? It is only suppose to show fields that have been changed by humans (even the developer) not systemd defaults. JBG -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] macro: allow assert_se() assertions to also be optimized when NDEBUG is set
replaces log with assert() to remove strings.
saves 3kB from text section of systemd.
---
src/shared/macro.h | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 7f89951..8cbff01 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -212,17 +212,21 @@ static inline unsigned long ALIGN_POWER2(unsigned long u)
{
(__x / __y + !!(__x % __y));\
})
-#define assert_se(expr) \
-do {\
-if (_unlikely_(!(expr)))\
-log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
-} while (false) \
-
/* We override the glibc assert() here. */
#undef assert
#ifdef NDEBUG
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+abort();\
+} while (false)
#define assert(expr) do {} while(false)
#else
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
+} while (false)
#define assert(expr) assert_se(expr)
#endif
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Drop systemd-ui
On Mon, Mar 30, 2015 at 8:34 PM, Andrei Borzenkov arvidj...@gmail.com wrote: В Mon, 30 Mar 2015 19:30:02 -0700 Shawn Landden shawnland...@gmail.com пишет: What do you feel is missing from systemctl show? It is only suppose to show fields that have been changed by humans (even the developer) not systemd defaults. From its appearance it was supposed to show current unit properties in machine parseable format. Are you sure you do not confused it with systemd cat? Yes I am sure, I wrote systemctl cat. -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set
On Thu, Mar 26, 2015 at 5:47 PM, Djalal Harouni tix...@opendz.org wrote:
On Fri, Mar 27, 2015 at 12:30:53AM +0100, Tom Gundersen wrote:
On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
lenn...@poettering.net wrote:
On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
Will result in slightly smaller binaries, and cuts out the branch, even if
the expression is still executed.
I am sorry, but the whole point of assert_se() is that it has side
effects. That's why it is called that way (the se suffix stands for
side effects), and is different from assert(). You are supposed to
use it whenever the code enclosed in it shall not be suppressed if
NDEBUG is defined. This patch of yours breaks that whole logic!
Hm, am I reading the patch wrong? It looks good to me. It is simply
dropping the branching and logging in the NDEBUG case, but the
expression itself is still evaluated.
Yep but it seems that abort() will never be called,
log_assert_failed() = abort()
And the logging mechanism is also one of those side effects. IMO unless
there are real valid reasons for any change to these macors... changing
anything here will just bring more bugs that we may never notice.
Those are the side effects of assert(), the side effects of the
expression are still evaluated.
So in the NDEBUG case assert_se(foo()) becomes equivalent to
foo(), which I guess makes sense (though I doubt it makes much of a
difference).
-t
---
src/shared/macro.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 7f89951..02219ea 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned
long u) {
(__x / __y + !!(__x % __y));\
})
-#define assert_se(expr) \
-do {\
-if (_unlikely_(!(expr)))\
-log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
-} while (false) \
-
/* We override the glibc assert() here. */
#undef assert
#ifdef NDEBUG
+#define assert_se(expr) do {expr} while(false)
#define assert(expr) do {} while(false)
#else
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
+} while (false)
#define assert(expr) assert_se(expr)
#endif
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Djalal Harouni
http://opendz.org
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set
On Thu, Mar 26, 2015 at 1:19 AM, Lennart Poettering
lenn...@poettering.net wrote:
On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
Will result in slightly smaller binaries, and cuts out the branch, even if
the expression is still executed.
I am sorry, but the whole point of assert_se() is that it has side
effects. That's why it is called that way (the se suffix stands for
side effects), and is different from assert(). You are supposed to
use it whenever the code enclosed in it shall not be suppressed if
NDEBUG is defined. This patch of yours breaks that whole logic!
+#define assert_se(expr) do {expr} while(false)
#define assert(expr) do {} while(false)
No it does not. see {expr}, it still is doing the side effect. It
just doesn't check if the return value is as expected.
Lennart
---
src/shared/macro.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 7f89951..02219ea 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long
u) {
(__x / __y + !!(__x % __y));\
})
-#define assert_se(expr) \
-do {\
-if (_unlikely_(!(expr)))\
-log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
-} while (false) \
-
/* We override the glibc assert() here. */
#undef assert
#ifdef NDEBUG
+#define assert_se(expr) do {expr} while(false)
#define assert(expr) do {} while(false)
#else
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
+} while (false)
#define assert(expr) assert_se(expr)
#endif
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] CODING_STYLE: this also help with unaligned memory accesses
On Thu, Mar 26, 2015 at 1:31 AM, Lennart Poettering
lenn...@poettering.net wrote:
On Tue, 24.03.15 11:16, Shawn Landden (sh...@churchofgit.com) wrote:
And those arches don't get much testing too.
---
CODING_STYLE | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index b687e72..8934954 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -14,7 +14,8 @@
- The destructors always unregister the object from the next bigger
object, not the other way around
-- To minimize strict aliasing violations, we prefer unions over casting
+- To minimize strict aliasing violations and unaligned memory accesses,
+ we prefer unions over casting
Unaligned memory accesses are an orthogonal problem really. I don't
see how the change above really makes sense?
casting often leads to unaligned memory accesses. Without casting the
compiler usually gets it right, but yeah this has little to do with
unions. I was confusing with having a packed and unpacked struct and
converting between the two like thus:
typedef struct {
uint8_t a
uint32_t b
} packed __attribute__((packed));
#if HAVE_FAST_UNALIGNED
#define unpacked packed
#elsif
typedef struct {
uint8_t a
// uint8_t __padding[3]; gcc will do this for you
uint32_t b
} unpacked;
#endif
foo get_foo(p *void) {
unpacked e;
e.a = (*packed)-a;
e.b = (*packed)-b;
return e;
}
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] timedatectl: check for getenv(TZDIR)
I liked having the DST information. It is a pity glibc doesn't export
this information.
If TZDIR is set, glibc will look there rather than /usr/share/zoneinfo.
See tzset(3).
---
src/timedate/timedatectl.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index 1d10c19..b10ab82 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -81,12 +81,18 @@ static void print_status_info(const StatusInfo *i) {
assert(i);
-/* Enforce the values of /etc/localtime */
+/* Enforce the values of /etc/localtime, or remote /etc/localtime*/
if (getenv(TZ)) {
fprintf(stderr, Warning: Ignoring the TZ variable.\n\n);
unsetenv(TZ);
}
+/* also use /usr/share/zoneinfo */
+if (getenv(TZDIR)) {
+fprintf(stderr, Warning: Ignoring the TZDIR variable. Using
system zoneinfo data.\n\n);
+unsetenv(TZDIR);
+}
+
r = setenv(TZ, i-timezone, false);
if (r 0) {
log_error_errno(errno, Failed to set TZ environment variable:
%m);
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd not honoring LD_LIBRARY_PATH?
On Fri, Mar 27, 2015 at 1:56 PM, Smith, Kenneth ksmit...@jaguarlandrover.com wrote: I am trying to start a service that requires using a library that has the same name as a preexisting one (it is a patched version). It is located in a different path so that if I want to start it at the command line I can do this: LD_LIBRARY_PATH=/opt/foo/lib /usr/bin/foo_c But when I do this: Environment=LD_LIBRARY_PATH=/opt/foo/lib ExecStart=/usr/bin/foo_c ldd shows foo_c is using the wrong library. Is there a way to make this work? -- Kind Regards, *Kenneth F. Smith* Linux C++ Engineer *T:* 971.256.9740 | *M:* 503-880-6256 *Email:* ksmit...@jaguarlandrover.com Jaguar Land Rover North America, LLC Open Source Technology Center 1419 Northwest 14th Avenue, Portland, OR 97209 JaguarUSA.com http://www.jaguarusa.com/index.html | LandRoverUSA.com http://www.landrover.com/us/en/lr/ Email: ksmit...@jaguarlandrover.com x...@jaguarlandrover.com PGP: RSA 2048/2048 979C6B958B89909D --- Business Details: Jaguar Land Rover Limited Registered Office: Abbey Road, Whitley, Coventry CV3 4LF Registered in England No: 1672070 This e-mail and any attachments contain confidential information for a specific individual and purpose. The information is private and privileged and intended solely for the use of the individual to whom it is addressed. If you are not the intended recipient, please e-mail us immediately. We apologise for any inconvenience caused but you are hereby notified that any disclosure, copying or distribution or the taking of any action in reliance on the information contained herein is strictly prohibited. You should be careful that this disclaimer doesn't come with any code you submit, or we can't look at it. This e-mail does not constitute an order for goods or services unless accompanied by an official purchase order. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] CODING_STYLE: this also help with unaligned memory accesses
And those arches don't get much testing too. --- CODING_STYLE | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CODING_STYLE b/CODING_STYLE index b687e72..8934954 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -14,7 +14,8 @@ - The destructors always unregister the object from the next bigger object, not the other way around -- To minimize strict aliasing violations, we prefer unions over casting +- To minimize strict aliasing violations and unaligned memory accesses, + we prefer unions over casting - For robustness reasons, destructors should be able to destruct half-initialized objects, too -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] timedatectl: check for getenv(TZDIR)
On Tue, Mar 24, 2015 at 11:32 AM, Kay Sievers k...@vrfy.org wrote:
On Tue, Mar 24, 2015 at 7:11 PM, Shawn Landden sh...@churchofgit.com wrote:
/* Enforce the values of /etc/localtime */
if (getenv(TZ)) {
-fprintf(stderr, Warning: Ignoring the TZ variable.\n\n);
+fprintf(stderr, Warning: Ignoring the %s variable.\n\n,
TZ);
What style is that? A second static string inserted with %s?
Wanto de-duplicate the first static string? :)
yep
-xstrftime(a, %a %Y-%m-%d %H:%M:%S UTC, gmtime_r(sec,
tm));
+xstrftime(a, %a %Y-%m-%d %H:%M:%S UTC, gmtime);
Why this change?
localtime_r() and gmtime_r() are called multiple times.
+ RTC in UTC by calling 'timedatectl
set-local-rtc 0'\n
+ For more details see
http://www.cl.cam.ac.uk/~mgk25/mswish/ut-rtc.html; ANSI_HIGHLIGHT_OFF .\n,
stdout);
Hmm, I find links to random web pages in error output a bit too
unconventional.
OK, but the page has been around for years and does tell Windows users
what registry entry to set to use UTC for their RTC.
Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] timedatectl: check for getenv(TZDIR)
I liked having the DST information. It is a pity glibc doesn't export
this information.
avoid calling gmtime_r() and localtime_r() twice
deduplicate some strings
v2
---
src/timedate/timedatectl.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index ab5c8a1..8073111 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -74,7 +74,7 @@ typedef struct StatusInfo {
static void print_status_info(const StatusInfo *i) {
char a[FORMAT_TIMESTAMP_MAX];
-struct tm tm;
+struct tm localtime, gmtime;
time_t sec;
bool have_time = false;
_cleanup_free_ char *zc = NULL, *zn = NULL;
@@ -84,10 +84,15 @@ static void print_status_info(const StatusInfo *i) {
/* Enforce the values of /etc/localtime */
if (getenv(TZ)) {
-fprintf(stderr, Warning: Ignoring the TZ variable.\n\n);
+fprintf(stderr, Warning: Ignoring the %s variable.\n\n,
TZ);
unsetenv(TZ);
}
+if (getenv(TZDIR)) {
+fprintf(stderr, Warning: Ignoring the %s variable.\n\n,
TZDIR);
+unsetenv(TZDIR);
+}
+
r = setenv(TZ, i-timezone, false);
if (r 0) {
log_error_errno(errno, Failed to set TZ environment variable:
%m);
@@ -105,27 +110,30 @@ static void print_status_info(const StatusInfo *i) {
fprintf(stderr, Warning: Could not get time from timedated
and not operating locally.\n\n);
if (have_time) {
-xstrftime(a, %a %Y-%m-%d %H:%M:%S %Z, localtime_r(sec,
tm));
+localtime_r(sec, localtime);
+gmtime_r(sec, gmtime);
+
+xstrftime(a, %a %Y-%m-%d %H:%M:%S %Z, localtime);
printf( Local time: %.*s\n, (int) sizeof(a), a);
-xstrftime(a, %a %Y-%m-%d %H:%M:%S UTC, gmtime_r(sec, tm));
+xstrftime(a, %a %Y-%m-%d %H:%M:%S UTC, gmtime);
printf( Universal time: %.*s\n, (int) sizeof(a), a);
} else {
-printf( Local time: %s\n, n/a);
-printf( Universal time: %s\n, n/a);
+printf( Local time: %.*s\n, (int) strlen(n/a), n/a);
+printf( Universal time: %.*s\n, (int) strlen(n/a), n/a);
}
if (i-rtc_time 0) {
time_t rtc_sec;
rtc_sec = (time_t)(i-rtc_time / USEC_PER_SEC);
-xstrftime(a, %a %Y-%m-%d %H:%M:%S, gmtime_r(rtc_sec, tm));
+xstrftime(a, %a %Y-%m-%d %H:%M:%S, gmtime);
printf(RTC time: %.*s\n, (int) sizeof(a), a);
} else
printf(RTC time: %s\n, n/a);
if (have_time)
-xstrftime(a, %Z, %z, localtime_r(sec, tm));
+xstrftime(a, %Z, %z, localtime);
printf( Time zone: %s (%.*s)\n
NTP enabled: %s\n
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] timedatectl: check for getenv(TZDIR)
I liked having the DST information. It is a pity glibc doesn't export
this information.
v3
---
src/timedate/timedatectl.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index ab5c8a1..d529a0a 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -88,6 +88,11 @@ static void print_status_info(const StatusInfo *i) {
unsetenv(TZ);
}
+if (getenv(TZDIR)) {
+fprintf(stderr, Warning: Ignoring the TZDIR variable.\n\n);
+unsetenv(TZDIR);
+}
+
r = setenv(TZ, i-timezone, false);
if (r 0) {
log_error_errno(errno, Failed to set TZ environment variable:
%m);
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] timedatectl: check for getenv(TZDIR)
I liked having the DST information. It is a pity glibc doesn't export
this information.
---
src/timedate/timedatectl.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index ab5c8a1..8daae54 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -74,7 +74,7 @@ typedef struct StatusInfo {
static void print_status_info(const StatusInfo *i) {
char a[FORMAT_TIMESTAMP_MAX];
-struct tm tm;
+struct tm localtime, gmtime;
time_t sec;
bool have_time = false;
_cleanup_free_ char *zc = NULL, *zn = NULL;
@@ -84,13 +84,18 @@ static void print_status_info(const StatusInfo *i) {
/* Enforce the values of /etc/localtime */
if (getenv(TZ)) {
-fprintf(stderr, Warning: Ignoring the TZ variable.\n\n);
+fprintf(stderr, Warning: Ignoring the %s variable.\n\n,
TZ);
unsetenv(TZ);
}
+if (getenv(TZDIR)) {
+fprintf(stderr, Warning: Ignoring the %s variable.\n\n,
TZDIR);
+unsetenv(TZDIR);
+}
+
r = setenv(TZ, i-timezone, false);
if (r 0) {
-log_error_errno(errno, Failed to set TZ environment variable:
%m);
+log_error_errno(errno, Failed to set %s environment variable:
%m, TZ);
exit(EXIT_FAILURE);
}
tzset();
@@ -105,27 +110,30 @@ static void print_status_info(const StatusInfo *i) {
fprintf(stderr, Warning: Could not get time from timedated
and not operating locally.\n\n);
if (have_time) {
-xstrftime(a, %a %Y-%m-%d %H:%M:%S %Z, localtime_r(sec,
tm));
+localtime_r(sec, localtime);
+gmtime_r(sec, gmtime);
+
+xstrftime(a, %a %Y-%m-%d %H:%M:%S %Z, localtime);
printf( Local time: %.*s\n, (int) sizeof(a), a);
-xstrftime(a, %a %Y-%m-%d %H:%M:%S UTC, gmtime_r(sec, tm));
+xstrftime(a, %a %Y-%m-%d %H:%M:%S UTC, gmtime);
printf( Universal time: %.*s\n, (int) sizeof(a), a);
} else {
-printf( Local time: %s\n, n/a);
-printf( Universal time: %s\n, n/a);
+printf( Local time: %.*s\n, (int) strlen(n/a), n/a);
+printf( Universal time: %.*s\n, (int) strlen(n/a), n/a);
}
if (i-rtc_time 0) {
time_t rtc_sec;
rtc_sec = (time_t)(i-rtc_time / USEC_PER_SEC);
-xstrftime(a, %a %Y-%m-%d %H:%M:%S, gmtime_r(rtc_sec, tm));
+xstrftime(a, %a %Y-%m-%d %H:%M:%S, gmtime);
printf(RTC time: %.*s\n, (int) sizeof(a), a);
} else
printf(RTC time: %s\n, n/a);
if (have_time)
-xstrftime(a, %Z, %z, localtime_r(sec, tm));
+xstrftime(a, %Z, %z, localtime);
printf( Time zone: %s (%.*s)\n
NTP enabled: %s\n
@@ -142,7 +150,8 @@ static void print_status_info(const StatusInfo *i) {
mode can not be fully supported. It will
create various problems with time\n
zone changes and daylight saving time
adjustments. The RTC time is never updated,\n
it relies on external facilities to maintain
it. If at all possible, use\n
- RTC in UTC by calling 'timedatectl
set-local-rtc 0' ANSI_HIGHLIGHT_OFF .\n, stdout);
+ RTC in UTC by calling 'timedatectl
set-local-rtc 0'\n
+ For more details see
http://www.cl.cam.ac.uk/~mgk25/mswish/ut-rtc.html; ANSI_HIGHLIGHT_OFF .\n,
stdout);
}
static int show_status(sd_bus *bus, char **args, unsigned n) {
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set
Will result in slightly smaller binaries, and cuts out the branch, even if
the expression is still executed.
---
src/shared/macro.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 7f89951..02219ea 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u)
{
(__x / __y + !!(__x % __y));\
})
-#define assert_se(expr) \
-do {\
-if (_unlikely_(!(expr)))\
-log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
-} while (false) \
-
/* We override the glibc assert() here. */
#undef assert
#ifdef NDEBUG
+#define assert_se(expr) do {expr} while(false)
#define assert(expr) do {} while(false)
#else
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+log_assert_failed(#expr, __FILE__, __LINE__,
__PRETTY_FUNCTION__); \
+} while (false)
#define assert(expr) assert_se(expr)
#endif
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset
On Mon, Mar 23, 2015 at 4:42 AM, Stef Walter st...@redhat.com wrote: On 23.03.2015 12:11, Shawn Landden wrote: On Sun, Mar 22, 2015 at 10:32 PM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote: Hmm, so this is a convenience call. You could just set tm.tm_zone locally and use mktime() with the value retrieved by Timezone? Yeah, the time-api is awful with global variables, but that's not really our fault, is it? This would not work, as struct tm's .tm_zone field is not sufficient to indicate a timezone. The code would have to set $TZ and call tzset(). Given the simplicity of this I'd probably just merge Stef's patch... It *is* kinda nice given that the timezone database is constantly updated and having this exposed on the bus so that it is accessible remotely has the benefit that you get the actual timezone information in effect on the remote system, and not a possible out-of-date timezone from the local database. If you follow what I mean... The patch is COMPLETELY WRONG, as timezone offsets are differn't for differn't times due to DST. Even if this is taken into account there are all sorts of races with this information around DST switches. As with the TimeUSec and RTCTimeUSec properties, it's assumed the caller knows the information is out of date the instant they receive it. Your patch is a whole hour slow during daylight savings time. (The epoch, when you query the offset for, is standard time as it is during January.) But in that case I guess we shouldn't do SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE... Fixed. How about a LocalTimeUSec property, for those times that the application doesn't have an interest doing timezone calculations and just wants the local time on the other machine. That's fine with me. Although IMO it's quite unorthodox to have local time in an integer value counted since the epoch. snip Stef -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset
On Sun, Mar 22, 2015 at 10:32 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote:
Hmm, so this is a convenience call. You could just set tm.tm_zone
locally and use mktime() with the value retrieved by Timezone? Yeah,
the time-api is awful with global variables, but that's not really our
fault, is it?
This would not work, as struct tm's .tm_zone field is not sufficient
to indicate a timezone. The code would have to set $TZ and call tzset().
Given the simplicity of this I'd probably just merge Stef's
patch... It *is* kinda nice given that the timezone database is
constantly updated and having this exposed on the bus so that it is
accessible remotely has the benefit that you get the actual timezone
information in effect on the remote system, and not a possible
out-of-date timezone from the local database. If you follow what I mean...
The patch is COMPLETELY WRONG, as timezone offsets are differn't for
differn't times due to DST. Even if this is taken into account there
are all sorts of races
with this information around DST switches.
How about a
LocalTimeUSec property, for those times that the application doesn't
have an interest doing timezone calculations and just wants the local
time on the other machine. In any other case I think the client has to
do proper timezone calculations.
I could write a patch for a TZDataVersion field that holds the tzdata release.
I'm not really against this bus-call, but I also don't see the point.
There's much more information in a timezone file than the offset, so
why expose the offset but not the other data?
Thanks
David
static int property_get_ntp_sync(
sd_bus *bus,
const char *path,
@@ -440,7 +456,8 @@ static int method_set_timezone(sd_bus *bus,
sd_bus_message *m, void *userdata, s
LOG_MESSAGE(Changed time zone to '%s'., c-zone),
NULL);
-sd_bus_emit_properties_changed(bus, /org/freedesktop/timedate1,
org.freedesktop.timedate1, Timezone, NULL);
+sd_bus_emit_properties_changed(bus, /org/freedesktop/timedate1,
org.freedesktop.timedate1,
+ Timezone, LocalOffset, NULL);
return sd_bus_reply_method_return(m, NULL);
}
@@ -666,6 +683,7 @@ static int method_set_ntp(sd_bus *bus, sd_bus_message
*m, void *userdata, sd_bus
static const sd_bus_vtable timedate_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY(Timezone, s, NULL, offsetof(Context, zone),
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
+SD_BUS_PROPERTY(LocalOffset, t, property_get_local_offset, 0,
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY(LocalRTC, b, bus_property_get_bool,
offsetof(Context, local_rtc), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY(CanNTP, b, bus_property_get_bool,
offsetof(Context, can_ntp), 0),
SD_BUS_PROPERTY(NTP, b, bus_property_get_bool,
offsetof(Context, use_ntp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
--
2.3.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Liberty equality fraternity or death,
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] bootchart: remove duplicated code, prevent creating empty files
In Debian and rawhide Fedora, which have CONFIG_SCHEDSTATS=n,
bootchart creates empty files in /run/log before printing an error.
Stop doing that.
Moreover this duplicated part of the code doesn't even have error checking
so there is no error avoided by doing this early.
Reported-by: tfirg_ on IRC
Signed-off-by: Shawn Landden sh...@churchofgit.com
---
src/bootchart/bootchart.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index 99ffb86..71dffc9 100644
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -395,15 +395,6 @@ int main(int argc, char *argv[]) {
sampledata-sampletime = gettime_ns();
sampledata-counter = samples;
-if (!of (access(arg_output_path, R_OK|W_OK|X_OK) == 0)) {
-t = time(NULL);
-r = strftime(datestr, sizeof(datestr), %Y%m%d-%H%M,
localtime(t));
-assert_se(r 0);
-
-snprintf(output_file, PATH_MAX, %s/bootchart-%s.svg,
arg_output_path, datestr);
-of = fopen(output_file, we);
-}
-
if (sysfd 0)
sysfd = open(/sys, O_RDONLY|O_CLOEXEC);
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] timedatectl: fix when queried system has differn't timezone
Also allow getting time from time(2) when BUS_TRANSPORT_MACHINE.
---
src/timedate/timedatectl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index 9e04f8f..7c9bd11 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -106,14 +106,17 @@ static void print_status_info(const StatusInfo *i) {
/* Enforce the values of /etc/localtime */
if (getenv(TZ)) {
-fprintf(stderr, Warning: Ignoring the TZ variable. Reading
the system's time zone setting only.\n\n);
+fprintf(stderr, Warning: Ignoring the TZ variable.\n\n);
unsetenv(TZ);
}
+setenv(TZ, i-timezone, false);
+tzset();
+
if (i-time != 0) {
sec = (time_t) (i-time / USEC_PER_SEC);
have_time = true;
-} else if (arg_transport == BUS_TRANSPORT_LOCAL) {
+} else if (IN_SET(arg_transport, BUS_TRANSPORT_REMOTE,
BUS_TRANSPORT_MACHINE)) {
sec = time(NULL);
have_time = true;
} else
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] timedatectl: fix when queried system has differn't timezone
Also allow getting time from time(2) when BUS_TRANSPORT_MACHINE.
v2: check for error
---
src/timedate/timedatectl.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index 9e04f8f..44d329e 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -106,14 +106,21 @@ static void print_status_info(const StatusInfo *i) {
/* Enforce the values of /etc/localtime */
if (getenv(TZ)) {
-fprintf(stderr, Warning: Ignoring the TZ variable. Reading
the system's time zone setting only.\n\n);
+fprintf(stderr, Warning: Ignoring the TZ variable.\n\n);
unsetenv(TZ);
}
+r = setenv(TZ, i-timezone, false);
+if (r 0) {
+log_error_errno(errno, Failed to set TZ environment variable:
%m);
+exit(EXIT_FAILURE);
+}
+tzset();
+
if (i-time != 0) {
sec = (time_t) (i-time / USEC_PER_SEC);
have_time = true;
-} else if (arg_transport == BUS_TRANSPORT_LOCAL) {
+} else if (IN_SET(arg_transport, BUS_TRANSPORT_REMOTE,
BUS_TRANSPORT_MACHINE)) {
sec = time(NULL);
have_time = true;
} else
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] bootchart: more useful error message for common error
Reported-by: tfirg_ on IRC
Signed-off-by: Shawn Landden sh...@churchofgit.com
---
src/bootchart/store.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index 607cc5e..dfa681f 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -176,7 +176,7 @@ vmstat_next:
/* overall CPU utilization */
schedstat = openat(procfd, schedstat, O_RDONLY);
if (schedstat == -1) {
-log_error_errno(errno, Failed to open
/proc/schedstat: %m);
+log_error_errno(errno, Failed to open /proc/schedstat
(requires CONFIG_SCHEDSTATS=y in kernel config): %m);
exit(EXIT_FAILURE);
}
}
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset
On Mon, Mar 23, 2015 at 12:13 PM, Stef Walter st...@redhat.com wrote: Sorry about the encrypted email ... I hit the wrong button. On 23.03.2015 19:07, Shawn Landden wrote: On Mon, Mar 23, 2015 at 8:56 AM, Kay Sievers k...@vrfy.org wrote: On Mon, Mar 23, 2015 at 3:49 PM, Stef Walter st...@redhat.com wrote: On 23.03.2015 15:26, Kay Sievers wrote: On Mon, Mar 23, 2015 at 1:46 PM, David Herrmann dh.herrm...@gmail.com wrote: Hi On Mon, Mar 23, 2015 at 6:32 AM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote: Hmm, so this is a convenience call. You could just set tm.tm_zone locally and use mktime() with the value retrieved by Timezone? Yeah, the time-api is awful with global variables, but that's not really our fault, is it? This would not work, as struct tm's .tm_zone field is not sufficient to indicate a timezone. The code would have to set $TZ and call tzset(). I see. The Unix-way of doing things! Given the simplicity of this I'd probably just merge Stef's patch... It *is* kinda nice given that the timezone database is constantly updated and having this exposed on the bus so that it is accessible remotely has the benefit that you get the actual timezone information in effect on the remote system, and not a possible out-of-date timezone from the local database. If you follow what I mean... ..locale data is also updated regularly but we don't export locale-files on the bus ;) Anyway, if we add pseudo-redundant APIs, I'd go with a LocalTime field as Shawn suggested. This is what the bus-user is interested in, right? If they need more data (like DST), they should just parse zoneinfo. And with LocalTime we avoid any ambiguity regarding DST. Transmitting several absolute time stamps sounds wrong. Transmitting the offset sounds as wrong as tz_minuteswest is and it got killed for that reason. Why does anybody ever need anything else than the actual UTC time and the configured timezone of the machine? Yes, we do. In Cockpit we want to show the server's local time, in the server's timezone. This is similar to how GNOME shows you the local time in the local timezone. But we don't have easy access to libc and its zoneinfo file parser from javascript running in a browser. We do have access to DBus [0] But it is not systemd's task to cover for missing functionality in the cockpit architecture. We should not add redundant interfaces just provide data for a specific user. Even systemd can't reliably consume its own DBus timedated interface remotely. Even once timedatectl is fixed, the information will be wrong unless the version of zoneinfo and glibc are identical on both systems involved. This is hardly Cockpit specific. The data systemd provides over the bus is the the same as the running system provides to the local user: the UTC time + the time zone. Everything else should be a task of the presentation, in this case cockpit. The timezone timedated provides is in presentation format (ie: Australia/Tasmania), a non-standard identifier, rather than a actual information about the timezone. Unless all systems agree on these names and have identical versions of the data (ie: zoneinfo), identical algorithms for interpreting it (ie: glibc) it is impossible to correctly interpret it remotely. all that would be required would be a hash of the zoneinfo file There are several DBus interfaces that possible not remotable, whether intentionally or not. For example NetwortkManager has byte[4] arrays stored in uint32's that are completely unusable on a different architecture ... other DBus interfaces have local file names as properties ... as so on. We should mark the timedated interface as non-remotable or call out the remotability problems in the documentation. The problem really isn't this bad. With my patch to actually use the remove timezone setting things are relatively OK. as is documented in tzset() glibc falls back to UTC. Yeah, it's currently a mix of local vs. remote data. If timedatctl should work correctly on a remote host, it needs to fixed. I posted a patch for this. With the current DBus interface, the only way to reliably fix this is to exec timedatectl remotely on the other system and pipe the output data. See above. Stef -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset
On Mon, Mar 23, 2015 at 8:56 AM, Kay Sievers k...@vrfy.org wrote: On Mon, Mar 23, 2015 at 3:49 PM, Stef Walter st...@redhat.com wrote: On 23.03.2015 15:26, Kay Sievers wrote: On Mon, Mar 23, 2015 at 1:46 PM, David Herrmann dh.herrm...@gmail.com wrote: Hi On Mon, Mar 23, 2015 at 6:32 AM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote: Hmm, so this is a convenience call. You could just set tm.tm_zone locally and use mktime() with the value retrieved by Timezone? Yeah, the time-api is awful with global variables, but that's not really our fault, is it? This would not work, as struct tm's .tm_zone field is not sufficient to indicate a timezone. The code would have to set $TZ and call tzset(). I see. The Unix-way of doing things! Given the simplicity of this I'd probably just merge Stef's patch... It *is* kinda nice given that the timezone database is constantly updated and having this exposed on the bus so that it is accessible remotely has the benefit that you get the actual timezone information in effect on the remote system, and not a possible out-of-date timezone from the local database. If you follow what I mean... ..locale data is also updated regularly but we don't export locale-files on the bus ;) Anyway, if we add pseudo-redundant APIs, I'd go with a LocalTime field as Shawn suggested. This is what the bus-user is interested in, right? If they need more data (like DST), they should just parse zoneinfo. And with LocalTime we avoid any ambiguity regarding DST. Transmitting several absolute time stamps sounds wrong. Transmitting the offset sounds as wrong as tz_minuteswest is and it got killed for that reason. Why does anybody ever need anything else than the actual UTC time and the configured timezone of the machine? Yes, we do. In Cockpit we want to show the server's local time, in the server's timezone. This is similar to how GNOME shows you the local time in the local timezone. But we don't have easy access to libc and its zoneinfo file parser from javascript running in a browser. We do have access to DBus [0] But it is not systemd's task to cover for missing functionality in the cockpit architecture. We should not add redundant interfaces just provide data for a specific user. The data systemd provides over the bus is the the same as the running system provides to the local user: the UTC time + the time zone. Everything else should be a task of the presentation, in this case cockpit. So obviously we could invent a DBus service called timedated2 (or whatever) to do what we need ... but I figured I'd see if timedated could do what we needed first. I don't think so. Systemd covers operating system data, but I doubt it should provide remote glibc functionality. It seems that timedatectl itself needs information about remote local time, since when connecting remotely over DBus it gets the local time wrong. [1] Yeah, it's currently a mix of local vs. remote data. If timedatctl should work correctly on a remote host, it needs to fixed. I posted a patch for this. Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Liberty equality fraternity or death, Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] timedated: add LocalTimeUSec via dbus
On Mon, Mar 23, 2015 at 6:52 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Mon, Mar 23, 2015 at 04:24:38AM -0700, Shawn Landden wrote:
---
src/timedate/timedated.c | 24
1 file changed, 24 insertions(+)
diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index ca771d5..f83b99c 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -38,6 +38,7 @@
#include bus-common-errors.h
#include event-util.h
#include selinux-util.h
+#include time-util.h
#define NULL_ADJTIME_UTC 0.0 0 0\n0\nUTC\n
#define NULL_ADJTIME_LOCAL 0.0 0 0\n0\nLOCAL\n
@@ -361,6 +362,28 @@ static int property_get_time(
return sd_bus_message_append(reply, t, now(CLOCK_REALTIME));
}
+/* gmtime(LocalTimeUSec % USEC_PER_SEC) is useful as long as you ignore GNU
+ * extensions of tm_gmtoff and tm_zone, and their ourgrowth in strftime(3)
+ */
+static int property_get_local_time(
+sd_bus *bus,
+const char *path,
+const char *interface,
+const char *property,
+sd_bus_message *reply,
+void *userdata,
+sd_bus_error *error)
+{
+struct tm tm;
+struct timespec ts;
+usec_t t;
+
+t = now(CLOCK_REALTIME);
+(void)timespec_store(ts, t);
+assert_se(localtime_r(dummy, ts.tv_sec));
+return sd_bus_message_append(reply, t, t + (tm-tm_gmtoff *
USEC_PER_SEC));
+}
+
static int property_get_ntp_sync(
sd_bus *bus,
const char *path,
@@ -671,6 +694,7 @@ static const sd_bus_vtable timedate_vtable[] = {
SD_BUS_PROPERTY(NTP, b, bus_property_get_bool,
offsetof(Context, use_ntp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY(NTPSynchronized, b, property_get_ntp_sync, 0,
0),
SD_BUS_PROPERTY(TimeUSec, t, property_get_time, 0, 0),
+SD_BUS_PROPERTY(LocalTimeUSec, t, property_get_local_time, 0,
0),
SD_BUS_PROPERTY(RTCTimeUSec, t, property_get_rtc_time, 0, 0),
SD_BUS_METHOD(SetTime, xbb, NULL, method_set_time,
SD_BUS_VTABLE_UNPRIVILEGED),
SD_BUS_METHOD(SetTimezone, sb, NULL, method_set_timezone,
SD_BUS_VTABLE_UNPRIVILEGED)
This patch seems the most reasonable of the proposed solutions.
It answers the question What does the remote host think local time is?
unambigously.
The other patch proposed by Shawn mixes the remote UTC time with local
information about timezones, which doesn't seem as clean.
These are not for the same problem. We need both. I want to ship a
checksum of the zoneinfo file with the timezone, but its hard to do
this cleanly as using coreutils from C is a PITA.
Kay said:
Transmitting several absolute time stamps sounds wrong.
The second timestamp is what the user sees, so in some way it is a
very significant thing. It is the outcome of the remote knowledge
about the time and its config and timezone database. We aren't making
any promises about the accuracy of that, but just trying to accurately
convey that information. Seems fine to me to transmit this bit of
information.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fix compiler warning
On Sun, Mar 15, 2015 at 4:53 AM, Ronny Chevalier chevalier.ro...@gmail.com
wrote:
2015-03-14 17:54 GMT+01:00 Shawn Landden sh...@churchofgit.com:
On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier
chevalier.ro...@gmail.com
wrote:
2015-03-11 4:42 GMT+01:00 Shawn Landden sh...@churchofgit.com:
warning: pointer/integer type mismatch in conditional expression
---
src/shared/socket-util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 5820279..73e1177 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
return -EAFNOSUPPORT;
return ntohs(sa-sa.sa_family == AF_INET6 ?
- sa-in6.sin6_port :
- sa-in.sin_port);
+ (uint16_t)sa-in6.sin6_port :
+ (uint16_t)sa-in.sin_port);
}
Hi,
I don't see any warning, and the man (man netinet_in.h) says that
sin_port and sin6_port are of type in_port_t which is equivalent to
uint16_t.
in_port_t and in6_port_t. If the type returned by a ternary operator is
not
identical gcc-5 warns, even though they are both backed by uint16_t and
thus
there is no violation.
netinet/in.h does not provide in6_port_t according to the manpage. I
even check with the glibc and on master there is no mention of
in6_port_t.
Maybe you are using another libc?
You are right, but has been a bug in an early gcc-5, which quite a few
incorrect warnings.
Maybe I'm missing something?
Ronny
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6
On Sun, Mar 15, 2015 at 1:07 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
if we are going to have a function to fix up the deficiencies of
inet_pton(), better go all the way.
---
src/shared/in-addr-util.c | 17 +
src/shared/in-addr-util.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
index ade4012..b7532a6 100644
--- a/src/shared/in-addr-util.c
+++ b/src/shared/in-addr-util.c
@@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
in_addr_union *u, char **ret) {
return 0;
}
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
+struct in6_addr r = { { {
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
+} } };
+
+r.s6_addr32[3] = ipv4.s_addr;
+
+return r;
+}
+
int in_addr_from_string(int family, const char *s, void *ret) {
+struct in_addr v4mapped;
assert(s);
assert(ret);
@@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char *s,
void *ret) {
if (!IN_SET(family, AF_INET, AF_INET6))
return -EAFNOSUPPORT;
+if (family == AF_INET6)
+if (inet_pton(AF_INET, s, v4mapped) == 1) {
+*((struct in6_addr *)ret) =
in_addr_to_in6_addr(v4mapped);
+return 0;
+}
+
Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4 address.
Automatically converting seems like a way to entice confusion...
Except that the linux kernel supports binding to ipv4 this way, (see
ipv6(7)).
In the inet_pton man page:
BUGS
AF_INET6 does not recognize IPv4 addresses. An explicit IPv4-mapped
IPv6 address must be supplied in src instead.
Zbyszek
errno = 0;
if (inet_pton(family, s, ret) = 0)
return errno ? -errno : -EINVAL;
diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
index 8b3a07c..f140ec9 100644
--- a/src/shared/in-addr-util.h
+++ b/src/shared/in-addr-util.h
@@ -37,6 +37,7 @@ int in_addr_equal(int family, const union
in_addr_union *a, const union in_addr_
int in_addr_prefix_intersect(int family, const union in_addr_union *a,
unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
prefixlen);
int in_addr_to_string(int family, const union in_addr_union *u, char
**ret);
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
int in_addr_from_string(int family, const char *s, void *ret);
int in_addr_from_string_auto(const char *s, int *family, union
in_addr_union *ret);
unsigned char in_addr_netmask_to_prefixlen(const struct in_addr *addr);
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6
On Sun, Mar 15, 2015 at 1:36 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Sun, Mar 15, 2015 at 01:28:05PM -0700, Shawn Landden wrote:
On Sun, Mar 15, 2015 at 1:07 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
if we are going to have a function to fix up the deficiencies of
inet_pton(), better go all the way.
---
src/shared/in-addr-util.c | 17 +
src/shared/in-addr-util.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
index ade4012..b7532a6 100644
--- a/src/shared/in-addr-util.c
+++ b/src/shared/in-addr-util.c
@@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
in_addr_union *u, char **ret) {
return 0;
}
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
+struct in6_addr r = { { {
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
+} } };
+
+r.s6_addr32[3] = ipv4.s_addr;
+
+return r;
+}
+
int in_addr_from_string(int family, const char *s, void *ret) {
+struct in_addr v4mapped;
assert(s);
assert(ret);
@@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char
*s,
void *ret) {
if (!IN_SET(family, AF_INET, AF_INET6))
return -EAFNOSUPPORT;
+if (family == AF_INET6)
+if (inet_pton(AF_INET, s, v4mapped) == 1) {
+*((struct in6_addr *)ret) =
in_addr_to_in6_addr(v4mapped);
+return 0;
+}
+
Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4
address.
Automatically converting seems like a way to entice confusion...
Except that the linux kernel supports binding to ipv4 this way, (see
ipv6(7)).
In the inet_pton man page:
BUGS
AF_INET6 does not recognize IPv4 addresses. An explicit
IPv4-mapped
IPv6 address must be supplied in src instead.
Right, but looking at how in_addr_from_string is used, we usually call it
with an explicit family, and if it parses as ipv4, assume the family is
ipv4,
otherwise parse it as ipv6. So your change would either be a noop (if
the check for ipv4 syntax was already done before), or would mess up this
detection.
What scenario are you trying to solve?
nothing is particular, I was just trying to give myself a reason not to
refactor these to just call inet_pton() directly as without this change the
wrapper function does nothing except check for NULL.
Zbyszek
Zbyszek
errno = 0;
if (inet_pton(family, s, ret) = 0)
return errno ? -errno : -EINVAL;
diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
index 8b3a07c..f140ec9 100644
--- a/src/shared/in-addr-util.h
+++ b/src/shared/in-addr-util.h
@@ -37,6 +37,7 @@ int in_addr_equal(int family, const union
in_addr_union *a, const union in_addr_
int in_addr_prefix_intersect(int family, const union in_addr_union
*a,
unsigned aprefixlen, const union in_addr_union *b, unsigned
bprefixlen);
int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
prefixlen);
int in_addr_to_string(int family, const union in_addr_union *u, char
**ret);
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
int in_addr_from_string(int family, const char *s, void *ret);
int in_addr_from_string_auto(const char *s, int *family, union
in_addr_union *ret);
unsigned char in_addr_netmask_to_prefixlen(const struct in_addr
*addr);
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] path-lookup: use secure_getenv()
All these except user_data_home_dir() are certainly vectors for
arbitrary code execution. These should use secure_getenv()
---
src/shared/path-lookup.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index fbf46cd..0fb86df 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -33,7 +33,7 @@ int user_config_home(char **config_home) {
const char *e;
char *r;
-e = getenv(XDG_CONFIG_HOME);
+e = secure_getenv(XDG_CONFIG_HOME);
if (e) {
r = strappend(e, /systemd/user);
if (!r)
@@ -44,7 +44,7 @@ int user_config_home(char **config_home) {
} else {
const char *home;
-home = getenv(HOME);
+home = secure_getenv(HOME);
if (home) {
r = strappend(home, /.config/systemd/user);
if (!r)
@@ -62,7 +62,7 @@ int user_runtime_dir(char **runtime_dir) {
const char *e;
char *r;
-e = getenv(XDG_RUNTIME_DIR);
+e = secure_getenv(XDG_RUNTIME_DIR);
if (e) {
r = strappend(e, /systemd/user);
if (!r)
@@ -83,13 +83,13 @@ static int user_data_home_dir(char **dir, const char
*suffix) {
* suggests because we assume that that is a link to
* /etc/systemd/ anyway. */
-e = getenv(XDG_DATA_HOME);
+e = secure_getenv(XDG_DATA_HOME);
if (e)
res = strappend(e, suffix);
else {
const char *home;
-home = getenv(HOME);
+home = secure_getenv(HOME);
if (home)
res = strjoin(home, /.local/share, suffix, NULL);
else
@@ -146,7 +146,7 @@ static char** user_dirs(
if (user_runtime_dir(runtime_dir) 0)
return NULL;
-e = getenv(XDG_CONFIG_DIRS);
+e = secure_getenv(XDG_CONFIG_DIRS);
if (e) {
config_dirs = strv_split(e, :);
if (!config_dirs)
@@ -157,7 +157,7 @@ static char** user_dirs(
if (r 0)
return NULL;
-e = getenv(XDG_DATA_DIRS);
+e = secure_getenv(XDG_DATA_DIRS);
if (e)
data_dirs = strv_split(e, :);
else
@@ -248,7 +248,7 @@ int lookup_paths_init(
/* First priority is whatever has been passed to us via env
* vars */
-e = getenv(SYSTEMD_UNIT_PATH);
+e = secure_getenv(SYSTEMD_UNIT_PATH);
if (e) {
if (endswith(e, :)) {
e = strndupa(e, strlen(e) - 1);
@@ -340,7 +340,7 @@ int lookup_paths_init(
#ifdef HAVE_SYSV_COMPAT
/* /etc/init.d/ compatibility does not matter to users */
-e = getenv(SYSTEMD_SYSVINIT_PATH);
+e = secure_getenv(SYSTEMD_SYSVINIT_PATH);
if (e) {
p-sysvinit_path = path_split_and_make_absolute(e);
if (!p-sysvinit_path)
@@ -358,7 +358,7 @@ int lookup_paths_init(
return -ENOMEM;
}
-e = getenv(SYSTEMD_SYSVRCND_PATH);
+e = secure_getenv(SYSTEMD_SYSVRCND_PATH);
if (e) {
p-sysvrcnd_path = path_split_and_make_absolute(e);
if (!p-sysvrcnd_path)
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] fix compiler warning
On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier chevalier.ro...@gmail.com
wrote:
2015-03-11 4:42 GMT+01:00 Shawn Landden sh...@churchofgit.com:
warning: pointer/integer type mismatch in conditional expression
---
src/shared/socket-util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 5820279..73e1177 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
return -EAFNOSUPPORT;
return ntohs(sa-sa.sa_family == AF_INET6 ?
- sa-in6.sin6_port :
- sa-in.sin_port);
+ (uint16_t)sa-in6.sin6_port :
+ (uint16_t)sa-in.sin_port);
}
Hi,
I don't see any warning, and the man (man netinet_in.h) says that
sin_port and sin6_port are of type in_port_t which is equivalent to
uint16_t.
in_port_t and in6_port_t. If the type returned by a ternary operator is not
identical gcc-5 warns, even though they are both backed by uint16_t and
thus there is no violation.
Maybe I'm missing something?
Ronny
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/6] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c
---
src/udev/udev-builtin-usb_id.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index 6516d93..3c15b2f 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
#include ctype.h
#include fcntl.h
#include errno.h
+#include inttypes.h
#include udev.h
@@ -179,21 +180,20 @@ static int dev_if_packed_info(struct udev_device *dev,
char *ifs_str, size_t len
ifs_str[0] = '\0';
while (pos size strpos+7 len-2) {
-struct usb_interface_descriptor *desc;
+unsigned char *desc = buf[pos];
char if_str[8];
-desc = (struct usb_interface_descriptor *) buf[pos];
-if (desc-bLength 3)
+if (desc[offsetof(struct usb_interface_descriptor, bLength)]
3)
break;
-pos += desc-bLength;
+pos += desc[offsetof(struct usb_interface_descriptor,
bLength)];
-if (desc-bDescriptorType != USB_DT_INTERFACE)
+if (desc[offsetof(struct usb_interface_descriptor,
bDescriptorType)] != USB_DT_INTERFACE)
continue;
if (snprintf(if_str, 8, :%02x%02x%02x,
- desc-bInterfaceClass,
- desc-bInterfaceSubClass,
- desc-bInterfaceProtocol) != 7)
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceClass)],
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceSubClass)],
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceProtocol)]) != 7)
continue;
if (strstr(ifs_str, if_str) != NULL)
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 6/6] refactor in_addr_to_string()
---
src/resolve/resolved-dns-rr.c | 6 ++
src/shared/in-addr-util.c | 32 +++-
2 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c
index 78d9e4a..a73ccd7 100644
--- a/src/resolve/resolved-dns-rr.c
+++ b/src/resolve/resolved-dns-rr.c
@@ -527,13 +527,11 @@ int dns_resource_record_to_string(const DnsResourceRecord
*rr, char **ret) {
break;
case DNS_TYPE_A: {
-_cleanup_free_ char *x = NULL;
-
-r = in_addr_to_string(AF_INET, (const union in_addr_union*)
rr-a.in_addr, x);
+r = in_addr_to_string(AF_INET, (const union in_addr_union*)
rr-a.in_addr, t);
if (r 0)
return r;
-s = strjoin(k, , x, NULL);
+s = strjoin(k, , t, NULL);
if (!s)
return -ENOMEM;
break;
diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
index b7532a6..f23b343 100644
--- a/src/shared/in-addr-util.c
+++ b/src/shared/in-addr-util.c
@@ -22,6 +22,7 @@
#include arpa/inet.h
#include in-addr-util.h
+#include socket-util.h
int in_addr_is_null(int family, const union in_addr_union *u) {
assert(u);
@@ -179,31 +180,20 @@ int in_addr_prefix_next(int family, union in_addr_union
*u, unsigned prefixlen)
}
int in_addr_to_string(int family, const union in_addr_union *u, char **ret) {
-char *x;
-size_t l;
+union sockaddr_union sa;
-assert(u);
-assert(ret);
+sa.sa.sa_family = family;
-if (family == AF_INET)
-l = INET_ADDRSTRLEN;
-else if (family == AF_INET6)
-l = INET6_ADDRSTRLEN;
-else
-return -EAFNOSUPPORT;
+if (sa.sa.sa_family == AF_INET) {
+sa.in.sin_addr = u-in;
-x = new(char, l);
-if (!x)
-return -ENOMEM;
+return sockaddr_pretty(sa.sa, (socklen_t)sizeof(sa.in),
false, false, ret);
+} else if (family == AF_INET6) {
+sa.in6.sin6_addr = u-in6;
-errno = 0;
-if (!inet_ntop(family, u, x, l)) {
-free(x);
-return errno ? -errno : -EINVAL;
-}
-
-*ret = x;
-return 0;
+return sockaddr_pretty(sa.sa, (socklen_t)sizeof(sa.in6),
false, false, ret);
+} else
+return -EAFNOSUPPORT;
}
struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6
if we are going to have a function to fix up the deficiencies of
inet_pton(), better go all the way.
---
src/shared/in-addr-util.c | 17 +
src/shared/in-addr-util.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
index ade4012..b7532a6 100644
--- a/src/shared/in-addr-util.c
+++ b/src/shared/in-addr-util.c
@@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
in_addr_union *u, char **ret) {
return 0;
}
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
+struct in6_addr r = { { {
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
+} } };
+
+r.s6_addr32[3] = ipv4.s_addr;
+
+return r;
+}
+
int in_addr_from_string(int family, const char *s, void *ret) {
+struct in_addr v4mapped;
assert(s);
assert(ret);
@@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char *s, void
*ret) {
if (!IN_SET(family, AF_INET, AF_INET6))
return -EAFNOSUPPORT;
+if (family == AF_INET6)
+if (inet_pton(AF_INET, s, v4mapped) == 1) {
+*((struct in6_addr *)ret) =
in_addr_to_in6_addr(v4mapped);
+return 0;
+}
+
errno = 0;
if (inet_pton(family, s, ret) = 0)
return errno ? -errno : -EINVAL;
diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
index 8b3a07c..f140ec9 100644
--- a/src/shared/in-addr-util.h
+++ b/src/shared/in-addr-util.h
@@ -37,6 +37,7 @@ int in_addr_equal(int family, const union in_addr_union *a,
const union in_addr_
int in_addr_prefix_intersect(int family, const union in_addr_union *a,
unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
prefixlen);
int in_addr_to_string(int family, const union in_addr_union *u, char **ret);
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
int in_addr_from_string(int family, const char *s, void *ret);
int in_addr_from_string_auto(const char *s, int *family, union in_addr_union
*ret);
unsigned char in_addr_netmask_to_prefixlen(const struct in_addr *addr);
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/6] fix strict aliasing issues in src/udev/udev-ctrl.c
it is ironic that
The only purpose of this structure is to cast the structure pointer
passed in addr in order to avoid compiler warnings. See EXAMPLE below.
from bind(2)
---
src/udev/udev-ctrl.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
index c0c5981..61d3c5b 100644
--- a/src/udev/udev-ctrl.c
+++ b/src/udev/udev-ctrl.c
@@ -18,6 +18,7 @@
#include sys/socket.h
#include sys/un.h
+#include socket-util.h
#include udev.h
/* wire protocol magic must match */
@@ -55,7 +56,7 @@ struct udev_ctrl {
int refcount;
struct udev *udev;
int sock;
-struct sockaddr_un saddr;
+union sockaddr_union saddr;
socklen_t addrlen;
bool bound;
bool cleanup_socket;
@@ -94,9 +95,9 @@ struct udev_ctrl *udev_ctrl_new_from_fd(struct udev *udev,
int fd) {
if (r 0)
log_warning_errno(errno, could not set SO_PASSCRED: %m);
-uctrl-saddr.sun_family = AF_LOCAL;
-strscpy(uctrl-saddr.sun_path, sizeof(uctrl-saddr.sun_path),
/run/udev/control);
-uctrl-addrlen = offsetof(struct sockaddr_un, sun_path) +
strlen(uctrl-saddr.sun_path);
+uctrl-saddr.un.sun_family = AF_LOCAL;
+strscpy(uctrl-saddr.un.sun_path, sizeof(uctrl-saddr.un.sun_path),
/run/udev/control);
+uctrl-addrlen = offsetof(struct sockaddr_un, sun_path) +
strlen(uctrl-saddr.un.sun_path);
return uctrl;
}
@@ -108,10 +109,10 @@ int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) {
int err;
if (!uctrl-bound) {
-err = bind(uctrl-sock, (struct sockaddr *)uctrl-saddr,
uctrl-addrlen);
+err = bind(uctrl-sock, uctrl-saddr.sa, uctrl-addrlen);
if (err 0 errno == EADDRINUSE) {
-unlink(uctrl-saddr.sun_path);
-err = bind(uctrl-sock, (struct sockaddr
*)uctrl-saddr, uctrl-addrlen);
+unlink(uctrl-saddr.un.sun_path);
+err = bind(uctrl-sock, uctrl-saddr.sa,
uctrl-addrlen);
}
if (err 0) {
@@ -160,7 +161,7 @@ int udev_ctrl_cleanup(struct udev_ctrl *uctrl) {
if (uctrl == NULL)
return 0;
if (uctrl-cleanup_socket)
-unlink(uctrl-saddr.sun_path);
+unlink(uctrl-saddr.un.sun_path);
return 0;
}
@@ -249,7 +250,7 @@ static int ctrl_send(struct udev_ctrl *uctrl, enum
udev_ctrl_msg_type type, int
ctrl_msg_wire.intval = intval;
if (!uctrl-connected) {
-if (connect(uctrl-sock, (struct sockaddr *)uctrl-saddr,
uctrl-addrlen) 0) {
+if (connect(uctrl-sock, uctrl-saddr.sa, uctrl-addrlen)
0) {
err = -errno;
goto out;
}
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 5/6] network: fix strict aliasing issue
We shouldn't assume 64-bit arch with the way we do math either.
(although I will submit a patch to glibc to add a uint64_t union alias)
---
src/network/networkd-address.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 0be6165..4b7f451 100644
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -605,12 +605,12 @@ bool address_equal(Address *a1, Address *a2) {
}
case AF_INET6: {
-uint64_t *b1, *b2;
+uint32_t *b1, *b2;
-b1 = (uint64_t*)a1-in_addr.in6;
-b2 = (uint64_t*)a2-in_addr.in6;
+b1 = a1-in_addr.in6.s6_addr32[0];
+b2 = a2-in_addr.in6.s6_addr32[0];
-return (((b1[0] ^ b2[0]) | (b1[1] ^ b2[1])) == 0UL);
+return (((b1[0] ^ b2[0]) | (b1[1] ^ b2[1]) | (b1[2] ^ b2[2]) |
(b1[3] ^ b2[3])) == 0);
}
default:
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
)
+return -1;
+else
+return 0;
+}
+if (!b)
+return 1;
+
/* easy comparison to see if versions are identical */
if (streq_ptr(a, b)) return 0;
-char oldch1, oldch2;
-char abuf[strlen(a)+1], bbuf[strlen(b)+1];
-char *str1 = abuf, *str2 = bbuf;
-char * one, * two;
-int rc;
-int isnum;
-
strcpy(str1, a);
strcpy(str2, b);
diff --git a/src/shared/strv.c b/src/shared/strv.c
index ee45ad1..ed28b95 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -81,7 +81,14 @@ void strv_clear(char **l) {
}
void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..823e768
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,231 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Shawn Landden
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include ctype.h
+#include dirent.h
+#include sys/utsname.h
+#include linux/kexec.h
+
+#include bootspec.h
+#include strv.h
+#include fileio.h
+#include rpmvercmp.h
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s-conf);
+free(s);
+}
+
+static int bootspec_cmp(const void *left, const void *right) {
+const struct BootSpec *l = left,
+ *r = right;
+
+/* reverse sort to put highest version first */
+return rpmvercmp(r-version, l-version);
+}
+
+int bootspec_getkernels(struct BootSpec **ret, size_t *cl) {
+int r = 0;
+struct BootSpec *c;
+_cleanup_closedir_ DIR *d;
+struct dirent *dent;
+size_t i = 1, ii = 2;
+_cleanup_close_ int midfd = -1;
+ssize_t ss;
+char mid_char[32 + 1];
+sd_id128_t machine_id;
+
+midfd = open(/etc/machine-id, O_RDONLY);
+if (midfd 0)
+return -errno;
+ss = read(midfd, mid_char, sizeof(mid_char));
+if (ss 0)
+return -errno;
+else if (ss != 33)
+return -EBADF;
+if (mid_char[32] == '\n')
+mid_char[32] = '\0';
+else
+return -EBADF;
+r = sd_id128_from_string(mid_char, machine_id);
+if (r 0)
+return r;
+
+c = new0(struct BootSpec, i + 1);
+if (!c)
+return -ENOMEM;
+
+d = opendir(BOOTENTRIESDIR);
+if (!d) {
+if (errno == ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+struct BootSpec *bs;
+char *m, *l, *k;
+_cleanup_fclose_ FILE *f = NULL;
+
+if (!endswith(dent-d_name, .conf))
+continue;
+
+c = greedy_realloc0((void **)c, ii, i + 2, sizeof(struct
BootSpec));
+if (!c)
+return -ENOMEM;
+bs = c[i - 1];
+
+f = fopenat(dirfd(d), dent-d_name, r);
+if (!f)
+return -errno;
+
+r = read_full_stream(f, bs-conf, NULL);
+if (r 0)
+return r;
+
+for (m = bs-conf; ; m = k + 1) {
+if (m[0] == '#')
+continue;
+
+k = strchr(m, '\n');
+
+if (k)
+*k = '\0';
+else
+break;
+
+if ((l = startswith(m, title )))
+bs-title = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, version )))
+bs-version= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, machine-id
[systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
;
+char abuf[strlen(a)+1], bbuf[strlen(b)+1];
+char *str1 = abuf, *str2 = bbuf;
+char * one, * two;
+int rc;
+int isnum;
+
+if (!a) {
+if (b)
+return -1;
+else
+return 0;
+}
+if (!b)
+return 1;
+
/* easy comparison to see if versions are identical */
if (streq_ptr(a, b)) return 0;
-char oldch1, oldch2;
-char abuf[strlen(a)+1], bbuf[strlen(b)+1];
-char *str1 = abuf, *str2 = bbuf;
-char * one, * two;
-int rc;
-int isnum;
-
strcpy(str1, a);
strcpy(str2, b);
diff --git a/src/shared/strv.c b/src/shared/strv.c
index ee45ad1..ed28b95 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -81,7 +81,14 @@ void strv_clear(char **l) {
}
void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..cabd46c
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,231 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Shawn Landden
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include ctype.h
+#include dirent.h
+#include sys/utsname.h
+#include linux/kexec.h
+
+#include bootspec.h
+#include strv.h
+#include fileio.h
+#include rpmvercmp.h
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s-conf);
+free(s);
+}
+
+static int bootspec_cmp(const void *left, const void *right) {
+const struct BootSpec *l = left,
+ *r = right;
+
+/* reverse sort to put highest version first */
+return rpmvercmp(r-version, l-version);
+}
+
+int bootspec_getkernels(struct BootSpec **ret, size_t *cl) {
+int r = 0;
+struct BootSpec *c;
+_cleanup_closedir_ DIR *d;
+struct dirent *dent;
+size_t i = 1, ii = 2;
+_cleanup_close_ int midfd = -1;
+ssize_t ss;
+char mid_char[32 + 1];
+sd_id128_t machine_id;
+
+midfd = open(/etc/machine-id, O_RDONLY);
+if (midfd 0)
+return -errno;
+ss = read(midfd, mid_char, sizeof(mid_char));
+if (ss 0)
+return -errno;
+else if (ss != 33)
+return -EBADF;
+if (mid_char[32] == '\n')
+mid_char[32] = '\0';
+else
+return -EBADF;
+r = sd_id128_from_string(mid_char, machine_id);
+if (r 0)
+return r;
+
+c = new0(struct BootSpec, i + 1);
+if (!c)
+return -ENOMEM;
+
+d = opendir(BOOTENTRIESDIR);
+if (!d) {
+if (errno == ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+struct BootSpec *bs;
+char *m, *l, *k;
+_cleanup_fclose_ FILE *f = NULL;
+
+if (!endswith(dent-d_name, .conf))
+continue;
+
+c = greedy_realloc0((void **)c, ii, i + 2, sizeof(struct
BootSpec));
+if (!c)
+return -ENOMEM;
+bs = c[i - 1];
+
+f = fopenat(dirfd(d), dent-d_name, r);
+if (!f)
+return -errno;
+
+r = read_full_stream(f, bs-conf, NULL);
+if (r 0)
+return r;
+
+for (m = bs-conf; ; m = k + 1) {
+if (m[0] == '#')
+continue;
+
+k = strchr(m, '\n');
+
+if (k)
+*k = '\0';
+else
+break;
+
+if ((l = startswith(m, title )))
+bs-title = l + strspn(l, WHITESPACE
Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
On Wed, Mar 11, 2015 at 5:51 PM, Kay Sievers k...@vrfy.org wrote: On Thu, Mar 12, 2015 at 1:22 AM, Shawn Landden sh...@churchofgit.com wrote: Still use helper when Xen Dom0, to avoid duplicating some hairy code. I think the rbtree version was far more understandable as greedy_realloc0() is very messy to do correctly. Take fopenat() from lsof. Add opendirat() We have that in util.c :: xopendirat() Future: generate BootLoaderSpec files for other kernel install locations This approach duplicates, the potentially complex, boot manager kernel selection logic. The recent systemd-boot boot loader and efi stub loader which carries the kernel, the cmdline, the initrd in one single EFI binary will also not use any boot loader snippets, it will be discovered by the loader itself, which parses the PE/COFF files and looks for specific content. The snippets are meant to unify the boot loader *configuration*, but they do not mean that every bootable kernel will or should have one. There might be many ways for kernels to be found by the boot loader, the snippets are just one source for that. I'm not sure what exact problem this patch tries to solve, rebooting with kexec is faster than a full reboot. Currently we do not support kexec very well. Lennart asked for something like this, but perhaps we no longer want to support kexec loading? but it generally does not sound right to duplicate the boot loader selection/discovery/enumeration logic here. I don't really think we should do that, or will be able to catch with the boot loader up here. Thanks, Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Shawn Landden ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
On Wed, Mar 11, 2015 at 5:51 PM, Kay Sievers k...@vrfy.org wrote: On Thu, Mar 12, 2015 at 1:22 AM, Shawn Landden sh...@churchofgit.com wrote: Still use helper when Xen Dom0, to avoid duplicating some hairy code. I think the rbtree version was far more understandable as greedy_realloc0() is very messy to do correctly. Take fopenat() from lsof. Add opendirat() We have that in util.c :: xopendirat() Future: generate BootLoaderSpec files for other kernel install locations This approach duplicates, the potentially complex, boot manager kernel selection logic. Hopefully it could be unified in some way. I find it disturbing that systemd will have a great deal of EFI-specific code. The recent systemd-boot boot loader and efi stub loader which carries the kernel, the cmdline, the initrd in one single EFI binary will also not use any boot loader snippets, it will be discovered by the loader itself, which parses the PE/COFF files and looks for specific content. The snippets are meant to unify the boot loader *configuration*, but they do not mean that every bootable kernel will or should have one. There might be many ways for kernels to be found by the boot loader, the snippets are just one source for that. I'm not sure what exact problem this patch tries to solve, but it generally does not sound right to duplicate the boot loader selection/discovery/enumeration logic here. I don't really think we should do that, or will be able to catch with the boot loader up here. Thanks, Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Shawn Landden ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
1: a is newer than b */
/*0: a and b are the same version */
/* -1: b is newer than a */
-int rpmvercmp(const char * a, const char * b)
-{
+int rpmvercmp(const char * a, const char * b) {
+char oldch1, oldch2;
+char abuf[strlen(a)+1], bbuf[strlen(b)+1];
+char *str1 = abuf, *str2 = bbuf;
+char * one, * two;
+int rc;
+int isnum;
+
+if (!a) {
+if (b)
+return -1;
+else
+return 0;
+}
+if (!b)
+return 1;
+
/* easy comparison to see if versions are identical */
if (streq_ptr(a, b)) return 0;
-char oldch1, oldch2;
-char abuf[strlen(a)+1], bbuf[strlen(b)+1];
-char *str1 = abuf, *str2 = bbuf;
-char * one, * two;
-int rc;
-int isnum;
-
strcpy(str1, a);
strcpy(str2, b);
diff --git a/src/shared/strv.c b/src/shared/strv.c
index ee45ad1..ed28b95 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -81,7 +81,14 @@ void strv_clear(char **l) {
}
void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..8194eaf
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,247 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Shawn Landden
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include ctype.h
+#include dirent.h
+#include sys/utsname.h
+#include linux/kexec.h
+
+#include bootspec.h
+#include strv.h
+#include fileio.h
+#include rpmvercmp.h
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s-conf);
+free(s);
+}
+
+static int bootspec_cmp(const void *left, const void *right) {
+const struct BootSpec *l = left,
+ *r = right;
+
+/* reverse sort to put highest version first */
+return rpmvercmp(r-version, l-version);
+}
+
+int bootspec_getkernels(struct BootSpec **ret, size_t *cl, int
*bootdir_fd_ret) {
+int r = 0;
+struct BootSpec *c;
+_cleanup_closedir_ DIR *d = NULL;
+struct dirent *dent;
+size_t i = 1, ii = 2;
+_cleanup_close_ int midfd = -1;
+int bootdir_fd = -1;
+ssize_t ss;
+char mid_char[32 + 1];
+sd_id128_t machine_id;
+
+assert(ret);
+assert(cl);
+
+/* we assume that the EFI partition is mounted
+ * and at /boot/efi or /boot
+ */
+errno = 0;
+if (access(/boot/efi/loader/entries, R_OK|X_OK) == 0)
+bootdir_fd = open(/boot/efi, O_DIRECTORY);
+else if (access(/boot/loader/entries, R_OK|X_OK) == 0)
+bootdir_fd = open(/boot, O_DIRECTORY);
+if (bootdir_fd 0)
+return IN_SET(errno, 0, ENOENT) ? 0 : -errno;
+
+midfd = open(/etc/machine-id, O_RDONLY);
+if (midfd 0)
+return -errno;
+ss = read(midfd, mid_char, sizeof(mid_char));
+if (ss 0)
+return -errno;
+else if (ss != 33)
+return -EBADF;
+if (mid_char[32] == '\n')
+mid_char[32] = '\0';
+else
+return -EBADF;
+r = sd_id128_from_string(mid_char, machine_id);
+if (r 0)
+return r;
+
+c = new0(struct BootSpec, i + 1);
+if (!c)
+return -ENOMEM;
+
+d = opendirat(bootdir_fd, loader/entries);
+if (!d) {
+if (errno == ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+struct BootSpec *bs;
+char *m, *l, *k;
+_cleanup_fclose_ FILE *f = NULL;
+
+if (!endswith(dent-d_name, .conf))
+continue;
+
+c = greedy_realloc0((void **)c, ii, i + 2, sizeof(struct
Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
On Wed, Mar 11, 2015 at 6:24 PM, Kay Sievers k...@vrfy.org wrote: On Thu, Mar 12, 2015 at 2:07 AM, Shawn Landden sh...@churchofgit.com wrote: On Wed, Mar 11, 2015 at 5:51 PM, Kay Sievers k...@vrfy.org wrote: On Thu, Mar 12, 2015 at 1:22 AM, Shawn Landden sh...@churchofgit.com wrote: Still use helper when Xen Dom0, to avoid duplicating some hairy code. I think the rbtree version was far more understandable as greedy_realloc0() is very messy to do correctly. Take fopenat() from lsof. Add opendirat() We have that in util.c :: xopendirat() Future: generate BootLoaderSpec files for other kernel install locations This approach duplicates, the potentially complex, boot manager kernel selection logic. The recent systemd-boot boot loader and efi stub loader which carries the kernel, the cmdline, the initrd in one single EFI binary will also not use any boot loader snippets, it will be discovered by the loader itself, which parses the PE/COFF files and looks for specific content. The snippets are meant to unify the boot loader *configuration*, but they do not mean that every bootable kernel will or should have one. There might be many ways for kernels to be found by the boot loader, the snippets are just one source for that. I'm not sure what exact problem this patch tries to solve, rebooting with kexec is faster than a full reboot. Currently we do not support kexec very well. Lennart asked for something like this, but perhaps we no longer want to support kexec loading? I kind of miss a description what this change is supposed to support in the end. It can't be described with replacing a call to a binary. Automatic searching and deciding what kernel to boot, and how to search for these kernels, and how all that should continue to work reliably in the longer run, while the boot loaders underneath improve and change; that is something we should define before and have a clear idea, before we copy only parts of that logic from the current tools. I thought you wrote a specification? freedesktop.org/wiki/Specifications/BootLoaderSpec/ Anyways, you should at least merge this patch: 1423865887-1507-1-git-send-email-sh...@churchofgit.com ( https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg28005.html ) Thanks, Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Shawn Landden ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
---
TODO | 2 -
man/systemd.socket.xml | 7 ++-
src/core/service.c | 41 -
src/libsystemd/sd-resolve/test-resolve.c | 2 +-
src/shared/socket-util.c | 76 +++-
src/shared/socket-util.h | 4 +-
src/timesync/timesyncd-server.h | 2 +-
7 files changed, 106 insertions(+), 28 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing multiple
parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or CGI
for this
-
* the install state probably shouldn't get confused by generated units, think
dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..6808179 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,12 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+
+paraFor IPv4 and IPv6 connections the varnameREMOTE_ADDR/varname
+environment variable will contain the remote IP, and
varnameREMOTE_PORT/varname
+will contain the remote port. This is the same as the format used by
CGI.
+For SOCK_RAW the port is the IP protocol./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..bcfce96 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
if (r 0)
goto fail;
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
if (!our_env) {
r = -ENOMEM;
goto fail;
@@ -1119,6 +1119,45 @@ static int service_spawn(
goto fail;
}
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) {
+_cleanup_free_ char *addr = NULL;
+char *t;
+int port;
+
+r = sockaddr_pretty(sa.sa, salen, true, false, addr);
+if (r 0)
+goto fail;
+
+t = strappend(REMOTE_ADDR=, addr);
+if (!t) {
+r = -ENOMEM;
+goto fail;
+}
+our_env[n_env++] = t;
+
+port = sockaddr_port(sa.sa);
+if (port 0) {
+r = port;
+goto fail;
+}
+
+if (asprintf((our_env + n_env++), REMOTE_PORT=%u,
port) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment, our_env,
NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int ret,
const struct addrin
for (i = ai; i; i = i-ai_next) {
_cleanup_free_ char *addr = NULL;
-assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
addr) == 0);
+assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
true, addr) == 0);
puts(addr);
}
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..0d87cb1 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char
**ret) {
return 0;
}
-return sockaddr_pretty(a-sockaddr.sa, a-size, false, ret);
+return sockaddr_pretty(a-sockaddr.sa, a-size, false, true, ret);
}
bool
[systemd-devel] [PATCH] fix strict aliasing issue in src/libsystemd-network/sd-dhcp-client.c
---
src/libsystemd-network/sd-dhcp-client.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/libsystemd-network/sd-dhcp-client.c
b/src/libsystemd-network/sd-dhcp-client.c
index 4224e01..a477ccc 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -1469,7 +1469,7 @@ static int client_receive_message_udp(sd_event_source *s,
int fd,
_cleanup_free_ DHCPMessage *message = NULL;
int buflen = 0, len, r;
const struct ether_addr zero_mac = { { 0, 0, 0, 0, 0, 0 } };
-const struct ether_addr *expected_chaddr = NULL;
+bool expect_chaddr;
uint8_t expected_hlen = 0;
assert(s);
@@ -1514,11 +1514,11 @@ static int client_receive_message_udp(sd_event_source
*s, int fd,
if (client-arp_type == ARPHRD_ETHER) {
expected_hlen = ETH_ALEN;
-expected_chaddr = (const struct ether_addr *)
client-mac_addr;
+expect_chaddr = true;
} else {
/* Non-ethernet links expect zero chaddr */
expected_hlen = 0;
- expected_chaddr = zero_mac;
+ expect_chaddr = false;
}
if (message-hlen != expected_hlen) {
@@ -1526,7 +1526,10 @@ static int client_receive_message_udp(sd_event_source
*s, int fd,
return 0;
}
-if (memcmp(message-chaddr[0], expected_chaddr, ETH_ALEN)) {
+if (memcmp(message-chaddr[0], expect_chaddr ?
+ (void *)client-mac_addr :
+ (void *)zero_mac,
+ETH_ALEN)) {
log_dhcp_client(client, received chaddr does not match
expected: ignoring);
return 0;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c
also switch to inttypes.h
---
src/udev/udev-builtin-usb_id.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index ab0d96e..b42b32e 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
#include ctype.h
#include fcntl.h
#include errno.h
+#include inttypes.h
#include udev.h
@@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device *dev,
char *ifs_str, size_t len
int pos = 0;
unsigned strpos = 0;
struct usb_interface_descriptor {
-u_int8_tbLength;
-u_int8_tbDescriptorType;
-u_int8_tbInterfaceNumber;
-u_int8_tbAlternateSetting;
-u_int8_tbNumEndpoints;
-u_int8_tbInterfaceClass;
-u_int8_tbInterfaceSubClass;
-u_int8_tbInterfaceProtocol;
-u_int8_tiInterface;
+uint8_tbLength;
+uint8_tbDescriptorType;
+uint8_tbInterfaceNumber;
+uint8_tbAlternateSetting;
+uint8_tbNumEndpoints;
+uint8_tbInterfaceClass;
+uint8_tbInterfaceSubClass;
+uint8_tbInterfaceProtocol;
+uint8_tiInterface;
} __attribute__((packed));
if (asprintf(filename, %s/descriptors,
udev_device_get_syspath(dev)) 0)
@@ -179,21 +180,20 @@ static int dev_if_packed_info(struct udev_device *dev,
char *ifs_str, size_t len
ifs_str[0] = '\0';
while (pos size strpos+7 len-2) {
-struct usb_interface_descriptor *desc;
+char *desc = buf[pos];
char if_str[8];
-desc = (struct usb_interface_descriptor *) buf[pos];
-if (desc-bLength 3)
+if (desc[offsetof(struct usb_interface_descriptor, bLength)]
3)
break;
-pos += desc-bLength;
+pos += desc[offsetof(struct usb_interface_descriptor,
bLength)];
-if (desc-bDescriptorType != USB_DT_INTERFACE)
+if (desc[offsetof(struct usb_interface_descriptor,
bDescriptorType)] != USB_DT_INTERFACE)
continue;
if (snprintf(if_str, 8, :%02x%02x%02x,
- desc-bInterfaceClass,
- desc-bInterfaceSubClass,
- desc-bInterfaceProtocol) != 7)
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceClass)],
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceSubClass)],
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceProtocol)]) != 7)
continue;
if (strstr(ifs_str, if_str) != NULL)
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix compiler warning
warning: pointer/integer type mismatch in conditional expression
---
src/shared/socket-util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 5820279..73e1177 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
return -EAFNOSUPPORT;
return ntohs(sa-sa.sa_family == AF_INET6 ?
- sa-in6.sin6_port :
- sa-in.sin_port);
+ (uint16_t)sa-in6.sin6_port :
+ (uint16_t)sa-in.sin_port);
}
int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool
translate_ipv6, bool include_port, char **ret) {
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix strict aliasing issues in src/udev/udev-ctrl.c
it is ironic that
The only purpose of this structure is to cast the structure pointer
passed in addr in order to avoid compiler warnings. See EXAMPLE below.
from bind(2)
---
src/udev/udev-ctrl.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
index c0c5981..61d3c5b 100644
--- a/src/udev/udev-ctrl.c
+++ b/src/udev/udev-ctrl.c
@@ -18,6 +18,7 @@
#include sys/socket.h
#include sys/un.h
+#include socket-util.h
#include udev.h
/* wire protocol magic must match */
@@ -55,7 +56,7 @@ struct udev_ctrl {
int refcount;
struct udev *udev;
int sock;
-struct sockaddr_un saddr;
+union sockaddr_union saddr;
socklen_t addrlen;
bool bound;
bool cleanup_socket;
@@ -94,9 +95,9 @@ struct udev_ctrl *udev_ctrl_new_from_fd(struct udev *udev,
int fd) {
if (r 0)
log_warning_errno(errno, could not set SO_PASSCRED: %m);
-uctrl-saddr.sun_family = AF_LOCAL;
-strscpy(uctrl-saddr.sun_path, sizeof(uctrl-saddr.sun_path),
/run/udev/control);
-uctrl-addrlen = offsetof(struct sockaddr_un, sun_path) +
strlen(uctrl-saddr.sun_path);
+uctrl-saddr.un.sun_family = AF_LOCAL;
+strscpy(uctrl-saddr.un.sun_path, sizeof(uctrl-saddr.un.sun_path),
/run/udev/control);
+uctrl-addrlen = offsetof(struct sockaddr_un, sun_path) +
strlen(uctrl-saddr.un.sun_path);
return uctrl;
}
@@ -108,10 +109,10 @@ int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) {
int err;
if (!uctrl-bound) {
-err = bind(uctrl-sock, (struct sockaddr *)uctrl-saddr,
uctrl-addrlen);
+err = bind(uctrl-sock, uctrl-saddr.sa, uctrl-addrlen);
if (err 0 errno == EADDRINUSE) {
-unlink(uctrl-saddr.sun_path);
-err = bind(uctrl-sock, (struct sockaddr
*)uctrl-saddr, uctrl-addrlen);
+unlink(uctrl-saddr.un.sun_path);
+err = bind(uctrl-sock, uctrl-saddr.sa,
uctrl-addrlen);
}
if (err 0) {
@@ -160,7 +161,7 @@ int udev_ctrl_cleanup(struct udev_ctrl *uctrl) {
if (uctrl == NULL)
return 0;
if (uctrl-cleanup_socket)
-unlink(uctrl-saddr.sun_path);
+unlink(uctrl-saddr.un.sun_path);
return 0;
}
@@ -249,7 +250,7 @@ static int ctrl_send(struct udev_ctrl *uctrl, enum
udev_ctrl_msg_type type, int
ctrl_msg_wire.intval = intval;
if (!uctrl-connected) {
-if (connect(uctrl-sock, (struct sockaddr *)uctrl-saddr,
uctrl-addrlen) 0) {
+if (connect(uctrl-sock, uctrl-saddr.sa, uctrl-addrlen)
0) {
err = -errno;
goto out;
}
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
On Mon, Mar 9, 2015 at 1:18 PM, Lennart Poettering lenn...@poettering.net
wrote:
On Mon, 09.03.15 13:09, Shawn Landden (sh...@churchofgit.com) wrote:
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+_cleanup_free_ char *addr = NULL;
+uint16_t port = (uint16_t)sockaddr_port(sa);
We try to avoid invoking functions in the same lines as we declare
variables.
Also, even though this cannot fail in this case, I think it would be
nicer, to make port an int, and check for 0 and return an error in
that case...
+
+r = sockaddr_pretty(sa.sa, salen, true,
false, addr);
+if (r 0)
+goto fail;
+
+if (!(our_env[n_env++] =
strappend(REMOTE_ADDR=, addr))) {
In newer code we try to avoid making assignments and doing if checks
in the same line.
Taking this out would be pretty gruesome because of use of the
post-increment operator.
-int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool
translate_ipv6, char **ret) {
+int sockaddr_port(const union sockaddr_union *sa) {
+assert_return(sa-sa.sa_family == AF_INET6 ||
+ sa-sa.sa_family == AF_INET,
+ -ENOTSUP);
assert_return() is a macro to use only for cases where there's a clear
programming error of the caller. But I am pretty sure that this is not
the case here. If it gets called on an non-AF_INET/AF_INET6 socket
then this should fail without logging.
Also I think an error code of EAFNOSUPPORT would be more appropriate.
Hmm, and I think this should probably take an actual struct
sockaddr, rather than a union sockaddr_union...
@@ -475,42 +485,40 @@ int sockaddr_pretty(const struct sockaddr *_sa,
socklen_t salen, bool translate_
switch (sa-sa.sa_family) {
-case AF_INET: {
-uint32_t a;
-
-a = ntohl(sa-in.sin_addr.s_addr);
-
-if (asprintf(p,
- %u.%u.%u.%u:%u,
- a 24, (a 16) 0xFF, (a 8)
0xFF, a 0xFF,
- ntohs(sa-in.sin_port)) 0)
-return -ENOMEM;
-
-break;
-}
-
+case AF_INET:
case AF_INET6: {
-static const unsigned char ipv4_prefix[] = {
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF
-};
-
-if (translate_ipv6 memcmp(sa-in6.sin6_addr,
ipv4_prefix, sizeof(ipv4_prefix)) == 0) {
-const uint8_t *a = sa-in6.sin6_addr.s6_addr+12;
+char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
+const char *addr;
+bool ipv4_mapped = false;
+
+if (inet_ntop(sa-sa.sa_family,
+ /* this field of the API is kinda
braindead,
+ * should take head of struct so it can
be passed the union...*/
+ sa-sa.sa_family == AF_INET6 ?
+sa-in6.sin6_addr :
+sa-in.sin_addr,
+ a, sizeof(a)) == NULL)
+ return -ENOMEM;
+
+/* glibc inet_ntop() presents v4-mapped addresses in
:::a.b.c.d form */
+if (translate_ipv6 sa-sa.sa_family == AF_INET6
strchr(a, '.')) {
+ipv4_mapped = true;
+addr = strempty(startswith(a, :::));
I think it would be a lot nicer to check the raw IP prefix as before,
and only then format things, then first formatting and then looking at
the string again.
We shouldn't bind us too closely to the precise formatting. I mean, if
one day libc decides to format the thing as ipv6 again, or with
uppercase hex chars, we shouldn't break.
+} else
+addr = a[0];
+
+if (include_port) {
+uint16_t port = (uint16_t)sockaddr_port(sa);
No function calls in variable declarations, please. Same rule as above
applies.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
---
TODO | 2 -
man/systemd.socket.xml | 6 ++-
src/core/service.c | 39 +++-
src/libsystemd/sd-resolve/test-resolve.c | 2 +-
src/shared/socket-util.c | 76 +++-
src/shared/socket-util.h | 4 +-
src/timesync/timesyncd-server.h | 2 +-
7 files changed, 103 insertions(+), 28 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing multiple
parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or CGI
for this
-
* the install state probably shouldn't get confused by generated units, think
dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..20f1e0c 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,11 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+paraFor IPv4 and IPv6 connections the varnameREMOTE_ADDR/varname
+environment variable will be set with remote IP, and
varnameREMOTE_PORT/varname
+environment variable set to the remote port, similar to CGI
+(for SOCK_RAW the port is the IP protocol)./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..7067a64 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
if (r 0)
goto fail;
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
if (!our_env) {
r = -ENOMEM;
goto fail;
@@ -1119,6 +1119,43 @@ static int service_spawn(
goto fail;
}
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+_cleanup_free_ char *addr = NULL;
+int port;
+
+r = sockaddr_pretty(sa.sa, salen, true, false, addr);
+if (r 0)
+goto fail;
+
+if (!(our_env[n_env++] = strappend(REMOTE_ADDR=,
addr))) {
+r = -ENOMEM;
+goto fail;
+}
+
+port = sockaddr_port(sa.sa);
+if (port 0) {
+r = port;
+goto fail;
+}
+
+if (asprintf(our_env[n_env++], REMOTE_PORT=%u, port)
0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment, our_env,
NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int ret,
const struct addrin
for (i = ai; i; i = i-ai_next) {
_cleanup_free_ char *addr = NULL;
-assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
addr) == 0);
+assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
true, addr) == 0);
puts(addr);
}
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..ca8ff39 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char
**ret) {
return 0;
}
-return sockaddr_pretty(a-sockaddr.sa, a-size, false, ret);
+return sockaddr_pretty(a-sockaddr.sa, a-size, false, true, ret);
}
bool socket_address_can_accept(const SocketAddress *a) {
@@ -466,7 +466,21 @@ bool
Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
On Mon, Mar 9, 2015 at 1:18 PM, Lennart Poettering lenn...@poettering.net
wrote:
On Mon, 09.03.15 13:09, Shawn Landden (sh...@churchofgit.com) wrote:
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+_cleanup_free_ char *addr = NULL;
+uint16_t port = (uint16_t)sockaddr_port(sa);
We try to avoid invoking functions in the same lines as we declare
variables.
Also, even though this cannot fail in this case, I think it would be
nicer, to make port an int, and check for 0 and return an error in
that case...
+
+r = sockaddr_pretty(sa.sa, salen, true,
false, addr);
+if (r 0)
+goto fail;
+
+if (!(our_env[n_env++] =
strappend(REMOTE_ADDR=, addr))) {
In newer code we try to avoid making assignments and doing if checks
in the same line.
-int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool
translate_ipv6, char **ret) {
+int sockaddr_port(const union sockaddr_union *sa) {
+assert_return(sa-sa.sa_family == AF_INET6 ||
+ sa-sa.sa_family == AF_INET,
+ -ENOTSUP);
assert_return() is a macro to use only for cases where there's a clear
programming error of the caller. But I am pretty sure that this is not
the case here. If it gets called on an non-AF_INET/AF_INET6 socket
then this should fail without logging.
Also I think an error code of EAFNOSUPPORT would be more appropriate.
Hmm, and I think this should probably take an actual struct
sockaddr, rather than a union sockaddr_union...
@@ -475,42 +485,40 @@ int sockaddr_pretty(const struct sockaddr *_sa,
socklen_t salen, bool translate_
switch (sa-sa.sa_family) {
-case AF_INET: {
-uint32_t a;
-
-a = ntohl(sa-in.sin_addr.s_addr);
-
-if (asprintf(p,
- %u.%u.%u.%u:%u,
- a 24, (a 16) 0xFF, (a 8)
0xFF, a 0xFF,
- ntohs(sa-in.sin_port)) 0)
-return -ENOMEM;
-
-break;
-}
-
+case AF_INET:
case AF_INET6: {
-static const unsigned char ipv4_prefix[] = {
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF
-};
-
-if (translate_ipv6 memcmp(sa-in6.sin6_addr,
ipv4_prefix, sizeof(ipv4_prefix)) == 0) {
-const uint8_t *a = sa-in6.sin6_addr.s6_addr+12;
+char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
+const char *addr;
+bool ipv4_mapped = false;
+
+if (inet_ntop(sa-sa.sa_family,
+ /* this field of the API is kinda
braindead,
+ * should take head of struct so it can
be passed the union...*/
+ sa-sa.sa_family == AF_INET6 ?
+sa-in6.sin6_addr :
+sa-in.sin_addr,
+ a, sizeof(a)) == NULL)
+ return -ENOMEM;
+
+/* glibc inet_ntop() presents v4-mapped addresses in
:::a.b.c.d form */
+if (translate_ipv6 sa-sa.sa_family == AF_INET6
strchr(a, '.')) {
+ipv4_mapped = true;
+addr = strempty(startswith(a, :::));
I think it would be a lot nicer to check the raw IP prefix as before,
and only then format things, then first formatting and then looking at
the string again.
We shouldn't bind us too closely to the precise formatting. I mean, if
one day libc decides to format the thing as ipv6 again, or with
uppercase hex chars, we shouldn't break.
https://tools.ietf.org/html/rfc5952#section-4.3 says lowercase-only
https://tools.ietf.org/html/rfc5952#section-5 specifies formatting of
ipv4-mapped addresses
and it is significantly longer that way, but sure, I can do it that way.
+} else
+addr = a[0];
+
+if (include_port) {
+uint16_t port = (uint16_t)sockaddr_port(sa);
No function calls in variable declarations, please. Same rule as above
applies.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http
[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
---
TODO | 2 -
man/systemd.socket.xml | 6 ++-
src/core/service.c | 35 +-
src/libsystemd/sd-resolve/test-resolve.c | 2 +-
src/shared/socket-util.c | 80 ++--
src/shared/socket-util.h | 4 +-
src/timesync/timesyncd-server.h | 2 +-
7 files changed, 88 insertions(+), 43 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing multiple
parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or CGI
for this
-
* the install state probably shouldn't get confused by generated units, think
dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..20f1e0c 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,11 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+paraFor IPv4 and IPv6 connections the varnameREMOTE_ADDR/varname
+environment variable will be set with remote IP, and
varnameREMOTE_PORT/varname
+environment variable set to the remote port, similar to CGI
+(for SOCK_RAW the port is the IP protocol)./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..89feec4 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
if (r 0)
goto fail;
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
if (!our_env) {
r = -ENOMEM;
goto fail;
@@ -1119,6 +1119,39 @@ static int service_spawn(
goto fail;
}
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+_cleanup_free_ char *addr = NULL;
+uint16_t port = (uint16_t)sockaddr_port(sa);
+
+r = sockaddr_pretty(sa.sa, salen, true, false, addr);
+if (r 0)
+goto fail;
+
+if (!(our_env[n_env++] = strappend(REMOTE_ADDR=,
addr))) {
+r = -ENOMEM;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ REMOTE_PORT=%u,
+ port) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment, our_env,
NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int ret,
const struct addrin
for (i = ai; i; i = i-ai_next) {
_cleanup_free_ char *addr = NULL;
-assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
addr) == 0);
+assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
true, addr) == 0);
puts(addr);
}
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..d7d34f8 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char
**ret) {
return 0;
}
-return sockaddr_pretty(a-sockaddr.sa, a-size, false, ret);
+return sockaddr_pretty(a-sockaddr.sa, a-size, false, true, ret);
}
bool socket_address_can_accept(const SocketAddress *a) {
@@ -466,7 +466,17 @@ bool socket_address_matches_fd(const SocketAddress *a, int
fd) {
return socket_address_equal(a, b);
Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
On Mon, Mar 9, 2015 at 9:22 AM, Lennart Poettering lenn...@poettering.net
wrote:
On Sun, 08.03.15 16:24, Shawn Landden (sh...@churchofgit.com) wrote:
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..6a690ac 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -22,6 +22,7 @@
#include errno.h
#include signal.h
#include unistd.h
+#include arpa/inet.h
#include async.h
#include manager.h
@@ -1119,6 +1120,52 @@ static int service_spawn(
goto fail;
}
+if (s-accept_socket.unit) {
Please use UNIT_DEREF() for this.
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+_cleanup_free_ char *remote_addr = NULL;
+char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+if (inet_ntop(sa.sa.sa_family,
+ /* this field of the API is kinda
braindead,
+ * should take head of struct so
it can be passed the union...*/
+ sa.sa.sa_family == AF_INET6 ?
+sa.in6.sin6_addr :
+sa.in.sin_addr,
+ a, sizeof(a)) == NULL) {
+r = -errno;
+goto fail;
+}
Hmm, so we already have sockaddr_pretty() in socket-util.c. Can't we
fold this logic into that? Maybe add a boolean param that indicates
whether to append the port number or not.
I'd rather not have code for this all over the place.
+
+if (asprintf(our_env + n_env++,
+ REMOTE_ADDR=%s,
+ /* musl and glibc inet_ntop()
present v4-mapped addresses in :::a.b.c.d form */
+ sa.sa.sa_family == AF_INET6
strchr(a, '.') ?
+ strempty(startswith(a,
:::)) :
+ a) 0) {
+r = -ENOMEM;
+goto fail;
+}
sockaddr_pretty() already has propery code for this, please use
that. Also, we don't care about non-glibc libcs anyway, hence please
no reference to that.
How about doing it this way in sockaddr_pretty() instead of rewriting
inet_ntop() to the fact that the man page does not say that glibc does this?
Otherwise I agree with everything in this review.
Then, we try to avoid asprintf() if we just want to concat strings,
please use strappend() or strjoin() for that.
+
+if (asprintf(our_env + n_env++,
+ REMOTE_PORT=%u,
+ ntohs(sa.sa.sa_family == AF_INET6 ?
+ sa.in6.sin6_port :
+ sa.in.sin_port)) 0) {
+r = -ENOMEM;
+goto fail;
To make this easy I think a new sockaddr_port() call in socket-util.c
would make sense that returns an int, in which it either encodes an
error or the converted port number.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
---
TODO | 2 --
man/systemd.socket.xml | 6 +-
src/core/service.c | 47 +++
3 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing multiple
parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or CGI
for this
-
* the install state probably shouldn't get confused by generated units, think
dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..20f1e0c 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,11 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+paraFor IPv4 and IPv6 connections the varnameREMOTE_ADDR/varname
+environment variable will be set with remote IP, and
varnameREMOTE_PORT/varname
+environment variable set to the remote port, similar to CGI
+(for SOCK_RAW the port is the IP protocol)./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..6a690ac 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -22,6 +22,7 @@
#include errno.h
#include signal.h
#include unistd.h
+#include arpa/inet.h
#include async.h
#include manager.h
@@ -1119,6 +1120,52 @@ static int service_spawn(
goto fail;
}
+if (s-accept_socket.unit) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+_cleanup_free_ char *remote_addr = NULL;
+char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+if (inet_ntop(sa.sa.sa_family,
+ /* this field of the API is kinda
braindead,
+ * should take head of struct so it can
be passed the union...*/
+ sa.sa.sa_family == AF_INET6 ?
+sa.in6.sin6_addr :
+sa.in.sin_addr,
+ a, sizeof(a)) == NULL) {
+r = -errno;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ REMOTE_ADDR=%s,
+ /* musl and glibc inet_ntop() present
v4-mapped addresses in :::a.b.c.d form */
+ sa.sa.sa_family == AF_INET6 strchr(a,
'.') ?
+ strempty(startswith(a, :::)) :
+ a) 0) {
+r = -ENOMEM;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ REMOTE_PORT=%u,
+ ntohs(sa.sa.sa_family == AF_INET6 ?
+ sa.in6.sin6_port :
+ sa.in.sin_port)) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment, our_env,
NULL);
if (!final_env) {
r = -ENOMEM;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp
it is trivial to fall back to our own timestamp v2: use now() v3: remove useless if () --- src/timedate/timedated.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 88d57e9..b8c586c 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -551,6 +551,9 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu if (c-use_ntp) return sd_bus_error_setf(error, BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, Automatic time synchronization is enabled); +/* this only gets used if dbus does not provide a timestamp */ +start = now(CLOCK_MONOTONIC); + r = sd_bus_message_read(m, xbb, utc, relative, interactive); if (r 0) return r; @@ -592,7 +595,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu r = sd_bus_message_get_monotonic_usec(m, start); if (r 0 r != -ENODATA) return r; -if (r = 0) +else timespec_store(ts, timespec_load(ts) + (now(CLOCK_MONOTONIC) - start)); /* Set system clock */ -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp
it is trivial to fall back to our own timestamp
v2: use now()
v3: remove useless if ()
v4: add comment
---
src/timedate/timedated.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 88d57e9..97b535f 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -551,6 +551,9 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m,
void *userdata, sd_bu
if (c-use_ntp)
return sd_bus_error_setf(error,
BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, Automatic time synchronization is
enabled);
+/* this only gets used if dbus does not provide a timestamp */
+start = now(CLOCK_MONOTONIC);
+
r = sd_bus_message_read(m, xbb, utc, relative, interactive);
if (r 0)
return r;
@@ -590,10 +593,11 @@ static int method_set_time(sd_bus *bus, sd_bus_message
*m, void *userdata, sd_bu
/* adjust ts for time spent in program */
r = sd_bus_message_get_monotonic_usec(m, start);
+/* when sd_bus_message_get_monotonic_usec() returns -ENODATA it does
not modify start */
if (r 0 r != -ENODATA)
return r;
-if (r = 0)
-timespec_store(ts, timespec_load(ts) + (now(CLOCK_MONOTONIC)
- start));
+
+timespec_store(ts, timespec_load(ts) + (now(CLOCK_MONOTONIC) -
start));
/* Set system clock */
if (clock_settime(CLOCK_REALTIME, ts) 0) {
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp
it is trivial to fall back to our own timestamp v2: use now() --- src/timedate/timedated.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 88d57e9..75b1f1b 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -551,6 +551,9 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu if (c-use_ntp) return sd_bus_error_setf(error, BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, Automatic time synchronization is enabled); +/* this only gets used if dbus does not provide a timestamp */ +start = now(CLOCK_MONOTONIC); + r = sd_bus_message_read(m, xbb, utc, relative, interactive); if (r 0) return r; @@ -592,7 +595,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu r = sd_bus_message_get_monotonic_usec(m, start); if (r 0 r != -ENODATA) return r; -if (r = 0) +if (r = 0 || r == -ENODATA) timespec_store(ts, timespec_load(ts) + (now(CLOCK_MONOTONIC) - start)); /* Set system clock */ -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp
it is trivial to fall back to our own timestamp --- src/timedate/timedated.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 88d57e9..7e47348 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -551,6 +551,13 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu if (c-use_ntp) return sd_bus_error_setf(error, BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, Automatic time synchronization is enabled); +/* this only gets used if dbus does not provide a timestamp + * (and ts gets overriden below) */ +r = clock_gettime(CLOCK_MONOTONIC, ts); +if (r 0) +return -errno; +start = timespec_load(ts); + r = sd_bus_message_read(m, xbb, utc, relative, interactive); if (r 0) return r; @@ -592,7 +599,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu r = sd_bus_message_get_monotonic_usec(m, start); if (r 0 r != -ENODATA) return r; -if (r = 0) +if (r = 0 || r == -ENODATA) timespec_store(ts, timespec_load(ts) + (now(CLOCK_MONOTONIC) - start)); /* Set system clock */ -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: Add REMOTE_IP environment variable for Accept=true
On Thu, Mar 5, 2015 at 3:18 AM, Lennart Poettering lenn...@poettering.net wrote: On Wed, 04.03.15 15:18, Shawn Landden (sh...@churchofgit.com) wrote: Can't this just use getpeername_pretty()? Then I can't force it to only ipv4 and ipv6. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Shawn Landden ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] core/socket: Add REMOTE_IP environment variable for Accept=true
Fix handling of abstract unix domain sockets too.
v2
---
TODO | 2 --
man/systemd.socket.xml | 5 -
src/core/service.c | 24
src/shared/socket-util.c | 25 +++--
4 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing multiple
parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or CGI
for this
-
* the install state probably shouldn't get confused by generated units, think
dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..8796d7b 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,10 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+paraFor IPv4 and IPv6 connections the varnameREMOTE_IP/varname
+environment variable will be set with remote IP and port seperated by a
+colon (for SOCK_RAW the port is the IP protocol)./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index a89ff3f..2ee4e11 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1119,6 +1119,30 @@ static int service_spawn(
goto fail;
}
+if (s-accept_socket.unit) {
+union sockaddr_union pn;
+socklen_t pnlen = sizeof(pn);
+_cleanup_free_ char *remote_addr = NULL;
+
+r = getpeername(s-socket_fd, pn.sa, pnlen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (pn.in.sin_family == AF_INET ||
+pn.in.sin_family == AF_INET6) {
+r = sockaddr_pretty(pn.sa, pnlen, true, remote_addr);
+if (r 0)
+goto fail;
+
+if (asprintf(our_env + n_env++, REMOTE_IP=%s,
remote_addr) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment, our_env,
NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..dbe2bf7 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -522,19 +522,32 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t
salen, bool translate_
return -ENOMEM;
} else if (sa-un.sun_path[0] == 0) {
+void *i;
/* abstract */
-/* FIXME: We assume we can print the
- * socket path here and that it hasn't
- * more than one NUL byte. That is
- * actually an invalid assumption */
-
+/* see unix(7), abstract is wierd */
+i = memchr(sa-un.sun_path + 1, '\0',
sizeof(sa-un.sun_path) - 1);
+if (i)
+for (i = (char *)i + 1;
+ (char *)i
sa-un.sun_path[sizeof(sa-un.sun_path)];
+ i = (char *)i + 1)
+if (*(char *)i != '\0') {
+p = strdup(abstract
unprintable);
+if (!p)
+return -ENOMEM;
+
+goto end;
+}
+
+/* no non-NUL bytes after second NUL (if any) */
p = new(char, sizeof(sa-un.sun_path)+1);
if (!p)
return -ENOMEM;
p[0] = '@';
memcpy(p+1, sa-un.sun_path+1,
sizeof(sa-un.sun_path)-1);
+
+/* make printable if there was no second NUL (i ==
NULL) */
p[sizeof(sa-un.sun_path)] = 0;
} else {
@@ -549,7 +562,7 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t
salen, bool translate_
Re: [systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c
On Wed, Mar 4, 2015 at 7:58 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Tue, Mar 03, 2015 at 04:21:30PM -0800, Shawn Landden wrote:
also switch to inttypes.h
---
src/udev/udev-builtin-usb_id.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/src/udev/udev-builtin-usb_id.c
b/src/udev/udev-builtin-usb_id.c
index ab0d96e..0223421 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
#include ctype.h
#include fcntl.h
#include errno.h
+#include inttypes.h
#include udev.h
@@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device
*dev, char *ifs_str, size_t len
int pos = 0;
unsigned strpos = 0;
struct usb_interface_descriptor {
-u_int8_tbLength;
-u_int8_tbDescriptorType;
-u_int8_tbInterfaceNumber;
-u_int8_tbAlternateSetting;
-u_int8_tbNumEndpoints;
-u_int8_tbInterfaceClass;
-u_int8_tbInterfaceSubClass;
-u_int8_tbInterfaceProtocol;
-u_int8_tiInterface;
+uint8_tbLength;
+uint8_tbDescriptorType;
+uint8_tbInterfaceNumber;
+uint8_tbAlternateSetting;
+uint8_tbNumEndpoints;
+uint8_tbInterfaceClass;
+uint8_tbInterfaceSubClass;
+uint8_tbInterfaceProtocol;
+uint8_tiInterface;
} __attribute__((packed));
if (asprintf(filename, %s/descriptors,
udev_device_get_syspath(dev)) 0)
@@ -179,21 +180,21 @@ static int dev_if_packed_info(struct udev_device
*dev, char *ifs_str, size_t len
ifs_str[0] = '\0';
while (pos size strpos+7 len-2) {
-struct usb_interface_descriptor *desc;
+struct usb_interface_descriptor desc;
char if_str[8];
-desc = (struct usb_interface_descriptor *) buf[pos];
-if (desc-bLength 3)
+memcpy(desc, buf[pos], sizeof(desc));
Copying it seems suboptimal. But is this actually an aliasing
violation? buf is a char array, and [1] says: a character type
may alias any other type.
Common misunderstanding about strict aliasing. if accessing as char * yes,
but not the other way around. See the C11 standard which makes it clear
(don't have page number off the top of my head...)
[1]
https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Optimize-Options.html#index-fstrict_002daliasing-825
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] core/socket: Add REMOTE_IP environment variable for Accept=true
Fix handling of abstract unix domain sockets too.
---
TODO | 2 --
man/systemd.socket.xml | 5 -
src/core/service.c | 24
src/shared/socket-util.c | 25 +++--
4 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing multiple
parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or CGI
for this
-
* the install state probably shouldn't get confused by generated units, think
dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..8796d7b 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,10 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+paraFor IPv4 and IPv6 connections the varnameREMOTE_IP/varname
+environment variable will be set with remote IP and port seperated by a
+colon (for SOCK_RAW the port is the IP protocol)./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index a89ff3f..0942072 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1119,6 +1119,30 @@ static int service_spawn(
goto fail;
}
+if (s-accept_socket.unit) {
+union sockaddr_union pn;
+socklen_t pnlen = sizeof(pn);
+_cleanup_free_ char *remote_addr = NULL;
+
+r = getpeername(s-socket_fd, pn.sa, pnlen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (pn.sa.sun_family == AF_INET ||
+pn.sa.sun_family == AF_INET6) {
+r = sockaddr_pretty(pn.sa, pnlen, true, remote_addr);
+if (r 0)
+goto fail;
+
+if (asprintf(our_env + n_env++, REMOTE_IP=%s,
remote_addr) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment, our_env,
NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..dbe2bf7 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -522,19 +522,32 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t
salen, bool translate_
return -ENOMEM;
} else if (sa-un.sun_path[0] == 0) {
+void *i;
/* abstract */
-/* FIXME: We assume we can print the
- * socket path here and that it hasn't
- * more than one NUL byte. That is
- * actually an invalid assumption */
-
+/* see unix(7), abstract is wierd */
+i = memchr(sa-un.sun_path + 1, '\0',
sizeof(sa-un.sun_path) - 1);
+if (i)
+for (i = (char *)i + 1;
+ (char *)i
sa-un.sun_path[sizeof(sa-un.sun_path)];
+ i = (char *)i + 1)
+if (*(char *)i != '\0') {
+p = strdup(abstract
unprintable);
+if (!p)
+return -ENOMEM;
+
+goto end;
+}
+
+/* no non-NUL bytes after second NUL (if any) */
p = new(char, sizeof(sa-un.sun_path)+1);
if (!p)
return -ENOMEM;
p[0] = '@';
memcpy(p+1, sa-un.sun_path+1,
sizeof(sa-un.sun_path)-1);
+
+/* make printable if there was no second NUL (i ==
NULL) */
p[sizeof(sa-un.sun_path)] = 0;
} else {
@@ -549,7 +562,7 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t
salen, bool translate_
Re: [systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c
Oh wait, I c, yes I had same question.
On Wed, Mar 4, 2015 at 8:07 PM, Shawn Landden sh...@churchofgit.com wrote:
On Wed, Mar 4, 2015 at 7:58 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Tue, Mar 03, 2015 at 04:21:30PM -0800, Shawn Landden wrote:
also switch to inttypes.h
---
src/udev/udev-builtin-usb_id.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/src/udev/udev-builtin-usb_id.c
b/src/udev/udev-builtin-usb_id.c
index ab0d96e..0223421 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
#include ctype.h
#include fcntl.h
#include errno.h
+#include inttypes.h
#include udev.h
@@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device
*dev, char *ifs_str, size_t len
int pos = 0;
unsigned strpos = 0;
struct usb_interface_descriptor {
-u_int8_tbLength;
-u_int8_tbDescriptorType;
-u_int8_tbInterfaceNumber;
-u_int8_tbAlternateSetting;
-u_int8_tbNumEndpoints;
-u_int8_tbInterfaceClass;
-u_int8_tbInterfaceSubClass;
-u_int8_tbInterfaceProtocol;
-u_int8_tiInterface;
+uint8_tbLength;
+uint8_tbDescriptorType;
+uint8_tbInterfaceNumber;
+uint8_tbAlternateSetting;
+uint8_tbNumEndpoints;
+uint8_tbInterfaceClass;
+uint8_tbInterfaceSubClass;
+uint8_tbInterfaceProtocol;
+uint8_tiInterface;
} __attribute__((packed));
if (asprintf(filename, %s/descriptors,
udev_device_get_syspath(dev)) 0)
@@ -179,21 +180,21 @@ static int dev_if_packed_info(struct udev_device
*dev, char *ifs_str, size_t len
ifs_str[0] = '\0';
while (pos size strpos+7 len-2) {
-struct usb_interface_descriptor *desc;
+struct usb_interface_descriptor desc;
char if_str[8];
-desc = (struct usb_interface_descriptor *) buf[pos];
-if (desc-bLength 3)
+memcpy(desc, buf[pos], sizeof(desc));
Copying it seems suboptimal. But is this actually an aliasing
violation? buf is a char array, and [1] says: a character type
may alias any other type.
Common misunderstanding about strict aliasing. if accessing as char *
yes, but not the other way around. See the C11 standard which makes it
clear (don't have page number off the top of my head...)
[1]
https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Optimize-Options.html#index-fstrict_002daliasing-825
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c
also switch to inttypes.h
---
src/udev/udev-builtin-usb_id.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index ab0d96e..0223421 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
#include ctype.h
#include fcntl.h
#include errno.h
+#include inttypes.h
#include udev.h
@@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device *dev,
char *ifs_str, size_t len
int pos = 0;
unsigned strpos = 0;
struct usb_interface_descriptor {
-u_int8_tbLength;
-u_int8_tbDescriptorType;
-u_int8_tbInterfaceNumber;
-u_int8_tbAlternateSetting;
-u_int8_tbNumEndpoints;
-u_int8_tbInterfaceClass;
-u_int8_tbInterfaceSubClass;
-u_int8_tbInterfaceProtocol;
-u_int8_tiInterface;
+uint8_tbLength;
+uint8_tbDescriptorType;
+uint8_tbInterfaceNumber;
+uint8_tbAlternateSetting;
+uint8_tbNumEndpoints;
+uint8_tbInterfaceClass;
+uint8_tbInterfaceSubClass;
+uint8_tbInterfaceProtocol;
+uint8_tiInterface;
} __attribute__((packed));
if (asprintf(filename, %s/descriptors,
udev_device_get_syspath(dev)) 0)
@@ -179,21 +180,21 @@ static int dev_if_packed_info(struct udev_device *dev,
char *ifs_str, size_t len
ifs_str[0] = '\0';
while (pos size strpos+7 len-2) {
-struct usb_interface_descriptor *desc;
+struct usb_interface_descriptor desc;
char if_str[8];
-desc = (struct usb_interface_descriptor *) buf[pos];
-if (desc-bLength 3)
+memcpy(desc, buf[pos], sizeof(desc));
+if (desc.bLength 3)
break;
-pos += desc-bLength;
+pos += desc.bLength;
-if (desc-bDescriptorType != USB_DT_INTERFACE)
+if (desc.bDescriptorType != USB_DT_INTERFACE)
continue;
if (snprintf(if_str, 8, :%02x%02x%02x,
- desc-bInterfaceClass,
- desc-bInterfaceSubClass,
- desc-bInterfaceProtocol) != 7)
+ desc.bInterfaceClass,
+ desc.bInterfaceSubClass,
+ desc.bInterfaceProtocol) != 7)
continue;
if (strstr(ifs_str, if_str) != NULL)
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 3/8] power: refactor the three power management binaries to src/power
On Thu, Feb 26, 2015 at 6:26 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: I'm not sure we want this... Can you add some justification? Do they share code? I found it confusing what parts each of these handled, and some code sharing is possible, but none is shared currently. This also eliminates one directory. All of these are handled by units: sleep.target shutdown.target etc... On Fri, Feb 20, 2015 at 02:31:00PM -0800, Shawn Landden wrote: --- Makefile.am | 6 +- src/core/shutdown.c | 420 - src/power/Makefile| 28 +++ This should be a symlink. To make all of these symlinks would be a much larger patch, but I can send such a patch src/power/shutdown.c | 420 + src/power/shutdownd.c | 461 ++ src/power/sleep.c | 219 ++ src/shutdownd/Makefile| 1 - src/shutdownd/shutdownd.c | 461 -- src/sleep/Makefile| 1 - src/sleep/sleep.c | 219 -- 10 files changed, 1131 insertions(+), 1105 deletions(-) delete mode 100644 src/core/shutdown.c create mode 100644 src/power/Makefile create mode 100644 src/power/shutdown.c create mode 100644 src/power/shutdownd.c create mode 100644 src/power/sleep.c delete mode 12 src/shutdownd/Makefile delete mode 100644 src/shutdownd/shutdownd.c delete mode 12 src/sleep/Makefile delete mode 100644 src/sleep/sleep.c It's better to use -M for such patches... Make it easier to see what is hapenning. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Shawn Landden ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [v1] shutdown: add kexec loading, ?avoid calling `kexec` binary unnessecarily
On Thu, Feb 26, 2015 at 6:22 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Thu, Feb 26, 2015 at 08:04:08AM +, Jan Janssen wrote:
Shawn Landden shawn at churchofgit.com writes:
void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
What are you trying to achieve here? I see no point in optimizing out
the *l
= NULL from strv_clear.
+entry-linux_loc = l + strspn(l,
WHITESPACE);
+else if ((l = startswith(m, initrd )))
+entry-initrd = l + strspn(l,
WHITESPACE);
You need to support more than one initrd per kernel, see
https://wiki.archlinux.org/index.php/Microcode for why. Also, I am
pretty
sure you can have a initrd=/path/to/initrd in the kernel options entry.
Since the efi bootloader just appends each given initrd to the kernel
command line.
All in all I am wondering why you need a rbtree for all this in the first
place? A simple hashmap should do just fine.
A hashmap does not keep order. But a simple array + qsort_safe() should
work too. I'm wary of introducing yet another data structure into systemd
which raises the bar for people editing the code later on or making
changes.
We need two operations: sorting kernels to list them, and picking (I
presume) the
latest kernel or the kernel with the given version. Both of those
operations
are done once over the lifetime of the program, so any speedup in using
a data structure should take into account the time to set up the structure.
Neither of those operations is speed sensitive, and the more common
operation
of picking a specific version can be done in O(n) over an array. So using
an rbtree will not save any time actually.
I was initially using a vector of pointers here for the same reasons you
reiterated, but I felt the use of greedy_realloc0() was messy and
error-prone. The rbtree does not require the use of realloc(). There is no
way to know how long the array needs to be from the start. Even the O(n)
you mention could be turned into O(logn) by using a binary search.
Also, you're not taking multi-boot into account (the machine-id field).
You're just discriminating based on the kernel version, but different
installations could have the same version field.
This high-level design questions should probably be addressed first...
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [v1] shutdown: add kexec loading, ?avoid calling `kexec` binary unnessecarily
On Fri, Feb 27, 2015 at 9:03 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
We need two operations: sorting kernels to list them, and picking (I
On Fri, Feb 27, 2015 at 08:58:04AM -0800, Shawn Landden wrote:
On Thu, Feb 26, 2015 at 6:22 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Thu, Feb 26, 2015 at 08:04:08AM +, Jan Janssen wrote:
Shawn Landden shawn at churchofgit.com writes:
void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
What are you trying to achieve here? I see no point in optimizing out
the *l
= NULL from strv_clear.
+entry-linux_loc = l + strspn(l,
WHITESPACE);
+else if ((l = startswith(m, initrd )))
+entry-initrd = l + strspn(l,
WHITESPACE);
You need to support more than one initrd per kernel, see
https://wiki.archlinux.org/index.php/Microcode for why. Also, I am
pretty
sure you can have a initrd=/path/to/initrd in the kernel options
entry.
Since the efi bootloader just appends each given initrd to the kernel
command line.
All in all I am wondering why you need a rbtree for all this in the
first
place? A simple hashmap should do just fine.
A hashmap does not keep order. But a simple array + qsort_safe() should
work too. I'm wary of introducing yet another data structure into
systemd
which raises the bar for people editing the code later on or making
changes.
presume) the
latest kernel or the kernel with the given version. Both of those
operations
are done once over the lifetime of the program, so any speedup in using
a data structure should take into account the time to set up the
structure.
Neither of those operations is speed sensitive, and the more common
operation
of picking a specific version can be done in O(n) over an array. So
using
an rbtree will not save any time actually.
I was initially using a vector of pointers here for the same reasons you
reiterated, but I felt the use of greedy_realloc0() was messy and
error-prone.
greedy_realloc0() is not that messy. And it would be just a few lines
of code. We have similar patterns in many other places, and
consistency is good.
The rbtree does not require the use of realloc(). There is no
way to know how long the array needs to be from the start. Even the O(n)
you mention could be turned into O(logn) by using a binary search.
Nope. The array needs to be sorted to do a binary search. So the
upfront cost of sorting kills any gain you get later on.
Oh I see, you don't have to sort in that case. But the code would be
longer, so perhaps I will sort in all cases.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] sd_id128_equal borked
_sd_pure_ static inline int sd_id128_equal(sd_id128_t a, sd_id128_t b) {
return memcmp(a, b, 16) == 0;
}
this should either be return memcmp(a, b, 16);
or return bool
--
---
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [v1] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
On Thu, Feb 26, 2015 at 12:04 AM, Jan Janssen medhe...@web.de wrote:
Shawn Landden shawn at churchofgit.com writes:
void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
What are you trying to achieve here? I see no point in optimizing out the
*l
= NULL from strv_clear.
+entry-linux_loc = l + strspn(l,
WHITESPACE);
+else if ((l = startswith(m, initrd )))
+entry-initrd = l + strspn(l,
WHITESPACE);
You need to support more than one initrd per kernel, see
https://wiki.archlinux.org/index.php/Microcode for why. Also, I am pretty
sure you can have a initrd=/path/to/initrd in the kernel options entry.
Since the efi bootloader just appends each given initrd to the kernel
command line.
I can't support more than one initrd per kernel with the kexec_file_load()
syscall, and if initrd on the commandline works, then it will still work
with this patch, so i don't need to change anything.
All in all I am wondering why you need a rbtree for all this in the first
place? A simple hashmap should do just fine.
Also, you're not taking multi-boot into account (the machine-id field).
You're just discriminating based on the kernel version, but different
installations could have the same version field.
fixed by testing that the machine-id is the same (I forgot this part of the
spec thanks). Is there anyway I should save defaults? Is there anything in
the spec that is missing? Perhaps it should specify how to save last-boot.
Jan
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
---
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [v3 1/4] man: these binaries are internal APIs
--- man/systemd-halt.service.xml | 1 - man/systemd-shutdownd.service.xml | 1 - man/systemd-suspend.service.xml | 1 - 3 files changed, 3 deletions(-) diff --git a/man/systemd-halt.service.xml b/man/systemd-halt.service.xml index c94e2a1..7e7f8f2 100644 --- a/man/systemd-halt.service.xml +++ b/man/systemd-halt.service.xml @@ -56,7 +56,6 @@ parafilenamesystemd-poweroff.service/filename/para parafilenamesystemd-reboot.service/filename/para parafilenamesystemd-kexec.service/filename/para -parafilename/usr/lib/systemd/systemd-shutdown/filename/para /refsynopsisdiv refsect1 diff --git a/man/systemd-shutdownd.service.xml b/man/systemd-shutdownd.service.xml index 756949c..051d2ab 100644 --- a/man/systemd-shutdownd.service.xml +++ b/man/systemd-shutdownd.service.xml @@ -52,7 +52,6 @@ refsynopsisdiv parafilenamesystemd-shutdownd.service/filename/para parafilenamesystemd-shutdownd.socket/filename/para -parafilename/usr/lib/systemd/systemd-shutdownd/filename/para /refsynopsisdiv refsect1 diff --git a/man/systemd-suspend.service.xml b/man/systemd-suspend.service.xml index a8beb86..8e3df5f 100644 --- a/man/systemd-suspend.service.xml +++ b/man/systemd-suspend.service.xml @@ -56,7 +56,6 @@ parafilenamesystemd-suspend.service/filename/para parafilenamesystemd-hibernate.service/filename/para parafilenamesystemd-hybrid-sleep.service/filename/para -parafilename/usr/lib/systemd/system-sleep/filename/para /refsynopsisdiv refsect1 -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] feature request: dlopen
shocked you into taking immediate action. my hope here is that you will realise the gravity and enormity of the situation that the software libre world faces right now, sufficient that you will give this absolute top priority above everything else that cannot be immediately put on hold. i am subscribed to this list on nomail, and will follow it on gmane. as you are experienced software developers i would not presume to interfere with how to go about dynamically-loading of libsystemd0, however if you would appreciate the benefit of my experience (which comes in part from one of the software libre world's more experienced unix portability experts, andrew tridgell), i will be more than happy to help as i can. l. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- --- Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] A use case for staged startup
On Sat, Feb 21, 2015 at 7:31 AM, Jeff Waugh j...@bethesignal.org wrote: Hi all, Marko Hoyer recently brought up the concept of a staged startup [1] on this list. I have a specific use case for some form of staging, though I don't know if it meets Marko's definition or requirements! Perhaps systemd can handle this already, but let's see... So, I've been building a systemd package for OpenWrt [2] to test on my little VoCore coin-sized MIPS machine. (Stay with me, the weird part is over.) The root filesystem is a read-only squashfs blob stored on the VoCore's generous (!) 8MB of flash memory. During initial testing, I was happy to boot up into a read-only environment, bring up a few tmpfs mount points, and then keep mucking around with systemd. But it's time to get serious. And everyone knows that serious means having a writeable root filesystem. OpenWrt uses overlayfs with JFFS2 as the top layer. but I'm just using tmpfs for now. (For some values of serious.) I wanted to make best use of systemd's built-in primitives, so here's what I've done: - default.target is symlinked to initrd.target in the read-only filesystem image - I've added some custom services to prepare all the mounts for the root switch (including one which changes the default.target symlink on the new, writeable root) Yes, I'm abusing systemd's idea of an initrd. Here's where it breaks down: - systemd dutifully starts all the services it knows about during the initrd.target run, because they're all right there on the read-only filesystem (and they fail a lot) - then systemd dutifully stops them all again to switch the root - and dutifully starts them all again once we're headed towards multi-user.target That's a *lot* of noise in the startup process! I did get the impression from the documentation that initrd.target was somehow special, but it makes complete sense that it's not. If I were using an initramfs, there wouldn't be any superfluous service files in the initramfs filesystem, and I'd be happy to know that systemd would behave *exactly* the same way it would elsewhere. One hacky idea I had to fix this: - Pull all of the systemd service symlinks out of the squashfs filesystem and store them in a tarball - Add specific symlinks to make the initrd stage works properly - In the pre-switch prepare service, unpack the tarball into the rw-mounted /sysroot Before anyone says it: No, using a real initramfs would be highly inappropriate. I do not want to store two copies of systemd and friends in 8MB of flash. It's hard enough with just the one! Is there an existing systemd solution to this problem? Is there a better way to go about it? You could try a bazillion bind mounts from the initrd to the target filesystem, and then calling switch_root(8) from util-linux, and you do not need systemctl like the previous post suggests, if you get the dependencies right. You might also look at overlayfs, which got merged in Linux 3.18. I think overlayfs will make this possible, and of course share your results! Thanks, Jeff [1] http://lists.freedesktop.org/archives/systemd-devel/2015-January/027688.html [2] https://github.com/jdub/openwrt-systemd ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- --- Shawn Landden ChurchOfGit.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/8] power: refactor the three power management binaries to src/power
---
Makefile.am | 6 +-
src/core/shutdown.c | 420 -
src/power/Makefile| 28 +++
src/power/shutdown.c | 420 +
src/power/shutdownd.c | 461 ++
src/power/sleep.c | 219 ++
src/shutdownd/Makefile| 1 -
src/shutdownd/shutdownd.c | 461 --
src/sleep/Makefile| 1 -
src/sleep/sleep.c | 219 --
10 files changed, 1131 insertions(+), 1105 deletions(-)
delete mode 100644 src/core/shutdown.c
create mode 100644 src/power/Makefile
create mode 100644 src/power/shutdown.c
create mode 100644 src/power/shutdownd.c
create mode 100644 src/power/sleep.c
delete mode 12 src/shutdownd/Makefile
delete mode 100644 src/shutdownd/shutdownd.c
delete mode 12 src/sleep/Makefile
delete mode 100644 src/sleep/sleep.c
diff --git a/Makefile.am b/Makefile.am
index ba63f68..52ec7ef 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2122,7 +2122,7 @@ systemd_update_done_LDADD = \
#
--
systemd_shutdownd_SOURCES = \
- src/shutdownd/shutdownd.c
+ src/power/shutdownd.c
systemd_shutdownd_LDADD = \
libsystemd-label.la \
@@ -2136,7 +2136,7 @@ dist_doc_DATA += \
systemd_shutdown_SOURCES = \
src/core/umount.c \
src/core/umount.h \
- src/core/shutdown.c \
+ src/power/shutdown.c \
src/core/mount-setup.c \
src/core/mount-setup.h \
src/core/killall.h \
@@ -2340,7 +2340,7 @@ systemd_sysctl_LDADD = \
#
--
systemd_sleep_SOURCES = \
- src/sleep/sleep.c
+ src/power/sleep.c
systemd_sleep_LDADD = \
libsystemd-shared.la
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
deleted file mode 100644
index 71f001a..000
--- a/src/core/shutdown.c
+++ /dev/null
@@ -1,420 +0,0 @@
-/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
-
-/***
- This file is part of systemd.
-
- Copyright 2010 ProFUSION embedded systems
-
- systemd is free software; you can redistribute it and/or modify it
- under the terms of the GNU Lesser General Public License as published by
- the Free Software Foundation; either version 2.1 of the License, or
- (at your option) any later version.
-
- systemd is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public License
- along with systemd; If not, see http://www.gnu.org/licenses/.
-***/
-
-#include sys/mman.h
-#include sys/types.h
-#include sys/reboot.h
-#include linux/reboot.h
-#include sys/wait.h
-#include sys/stat.h
-#include sys/mount.h
-#include sys/syscall.h
-#include fcntl.h
-#include dirent.h
-#include errno.h
-#include unistd.h
-#include signal.h
-#include stdbool.h
-#include stdlib.h
-#include string.h
-#include getopt.h
-
-#include missing.h
-#include log.h
-#include fileio.h
-#include umount.h
-#include util.h
-#include mkdir.h
-#include virt.h
-#include watchdog.h
-#include killall.h
-#include cgroup-util.h
-#include def.h
-#include switch-root.h
-#include strv.h
-
-#define FINALIZE_ATTEMPTS 50
-
-static char* arg_verb;
-
-static int parse_argv(int argc, char *argv[]) {
-enum {
-ARG_LOG_LEVEL = 0x100,
-ARG_LOG_TARGET,
-ARG_LOG_COLOR,
-ARG_LOG_LOCATION,
-};
-
-static const struct option options[] = {
-{ log-level, required_argument, NULL, ARG_LOG_LEVEL},
-{ log-target,required_argument, NULL, ARG_LOG_TARGET },
-{ log-color, optional_argument, NULL, ARG_LOG_COLOR},
-{ log-location, optional_argument, NULL, ARG_LOG_LOCATION },
-{}
-};
-
-int c, r;
-
-assert(argc = 1);
-assert(argv);
-
-/* - prevents getopt from permuting argv[] and moving the verb away
- * from argv[1]. Our interface to initrd promises it'll be there. */
-while ((c = getopt_long(argc, argv, -, options, NULL)) = 0)
-switch (c) {
-
-case ARG_LOG_LEVEL:
-r = log_set_max_level_from_string(optarg);
-if (r 0)
-log_error(Failed to parse log level %s,
ignoring., optarg);
-
-break;
-
-case ARG_LOG_TARGET:
-r = log_set_target_from_string(optarg);
-if (r 0)
-log_error(Failed to parse log target
[systemd-devel] [PATCH 6/8] add rbtree implamentation
from https://github.com/fbuihuu/libtree (LGPLv2.1+)
---
Makefile.am | 2 +
src/shared/rbtree.c | 482
src/shared/rbtree.h | 79 +
3 files changed, 563 insertions(+)
create mode 100644 src/shared/rbtree.c
create mode 100644 src/shared/rbtree.h
diff --git a/Makefile.am b/Makefile.am
index 52ec7ef..e0813a6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -902,6 +902,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/verbs.h \
src/shared/sigbus.c \
src/shared/sigbus.h \
+ src/shared/rbtree.c \
+ src/shared/rbtree.h \
src/shared/build.h \
src/shared/import-util.c \
src/shared/import-util.h
diff --git a/src/shared/rbtree.c b/src/shared/rbtree.c
new file mode 100644
index 000..6b77b0f
--- /dev/null
+++ b/src/shared/rbtree.c
@@ -0,0 +1,482 @@
+/*
+ * rbtree - Implements a red-black tree with parent pointers.
+ *
+ * Copyright (C) 2010-2014 Franck Bui-Huu fbui...@gmail.com
+ *
+ * This file is part of libtree which is free software; you can
+ * redistribute it and/or modify it under the terms of the GNU Lesser
+ * General Public License as published by the Free Software
+ * Foundation; either version 2.1 of the License, or (at your option)
+ * any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * See the LICENSE file for license rights and limitations.
+ */
+
+/*
+ * For recall a red-black tree has the following properties:
+ *
+ * 1. All nodes are either BLACK or RED
+ * 2. Leafs are BLACK
+ * 3. A RED node has BLACK children only
+ * 4. Path from a node to any leafs has the same number of BLACK nodes.
+ *
+ */
+#include rbtree.h
+
+/*
+ * Some helpers
+ */
+#ifdef UINTPTR_MAX
+
+static inline enum rb_color get_color(const struct rbtree_node *node)
+{
+ return node-parent 1;
+}
+
+static inline void set_color(enum rb_color color, struct rbtree_node *node)
+{
+ node-parent = (node-parent ~1UL) | color;
+}
+
+static inline struct rbtree_node *get_parent(const struct rbtree_node *node)
+{
+ return (struct rbtree_node *)(node-parent ~1UL);
+}
+
+static inline void set_parent(struct rbtree_node *parent, struct rbtree_node
*node)
+{
+ node-parent = (uintptr_t)parent | (node-parent 1);
+}
+
+#else
+
+static inline enum rb_color get_color(const struct rbtree_node *node)
+{
+ return node-color;
+}
+
+static inline void set_color(enum rb_color color, struct rbtree_node *node)
+{
+ node-color = color;
+}
+
+static inline struct rbtree_node *get_parent(const struct rbtree_node *node)
+{
+ return node-parent;
+}
+
+static inline void set_parent(struct rbtree_node *parent, struct rbtree_node
*node)
+{
+ node-parent = parent;
+}
+
+#endif /* UINTPTR_MAX */
+
+static inline int is_root(struct rbtree_node *node)
+{
+ return get_parent(node) == NULL;
+}
+
+static inline int is_black(struct rbtree_node *node)
+{
+ return get_color(node) == RB_BLACK;
+}
+
+static inline int is_red(struct rbtree_node *node)
+{
+ return !is_black(node);
+}
+
+/*
+ * Iterators
+ */
+static inline struct rbtree_node *get_first(struct rbtree_node *node)
+{
+ while (node-left)
+ node = node-left;
+ return node;
+}
+
+static inline struct rbtree_node *get_last(struct rbtree_node *node)
+{
+ while (node-right)
+ node = node-right;
+ return node;
+}
+
+struct rbtree_node *rbtree_first(const struct rbtree *tree)
+{
+ return tree-first;
+}
+
+struct rbtree_node *rbtree_last(const struct rbtree *tree)
+{
+ return tree-last;
+}
+
+struct rbtree_node *rbtree_next(const struct rbtree_node *node)
+{
+ struct rbtree_node *parent;
+
+ if (node-right)
+ return get_first(node-right);
+
+ while ((parent = get_parent(node)) parent-right == node)
+ node = parent;
+ return parent;
+}
+
+struct rbtree_node *rbtree_prev(const struct rbtree_node *node)
+{
+ struct rbtree_node *parent;
+
+ if (node-left)
+ return get_last(node-left);
+
+ while ((parent = get_parent(node)) parent-left == node)
+ node = parent;
+ return parent;
+}
+
+/*
+ * 'pparent' and 'is_left' are only used for insertions. Normally GCC
+ * will notice this and get rid of them for lookups.
+ */
+static inline struct rbtree_node *do_lookup(const struct rbtree_node *key,
+ const struct rbtree *tree,
+ struct rbtree_node **pparent,
+ int *is_left)
+{
+ struct rbtree_node *node = tree-root;
+
+ *pparent = NULL;
+ *is_left = 0;
+
+ while (node) {
+ int res = tree-cmp_fn(node, key);
+ if
[systemd-devel] [PATCH 8/8] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..8673957
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,195 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Shawn Landden
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include ctype.h
+#include dirent.h
+#include sys/utsname.h
+#include linux/kexec.h
+
+#include bootspec.h
+#include strv.h
+#include fileio.h
+#include rpmvercmp.h
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s-conf);
+free(s);
+}
+
+static int bootspec_cmp(const struct rbtree_node *left, const struct
rbtree_node *right) {
+struct BootSpec *l = rbtree_container_of( left, struct BootSpec, node),
+*r = rbtree_container_of(right, struct BootSpec, node);
+
+return rpmvercmp(l-version, r-version);
+}
+
+int kernel_bootloaderspec_readconf(struct rbtree *tree) {
+int r = 0;
+_cleanup_closedir_ DIR *entries;
+struct dirent *dir;
+
+rbtree_init(tree, bootspec_cmp, 0);
+
+entries = opendir(BOOTENTRIESDIR);
+if (!entries) {
+if (r == -ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (size_t i=0;(dir = readdir(entries));i++) {
+struct BootSpec *entry;
+/* compiler wont allow 256 + strlen(BOOTENTRIESDIR) here */
+char fn[512] = {BOOTENTRIESDIR}, *d_name =
fn[strlen(BOOTENTRIESDIR)],
+ *m, *l, *k;
+
+if (!endswith(dir-d_name, .conf)) {
+i--;
+continue;
+}
+
+entry = new0(struct BootSpec, 1);
+if (!entry)
+return -ENOMEM;
+
+(void)strcpy(d_name, dir-d_name);
+
+r = read_full_file(fn, entry-conf, NULL);
+if (r 0)
+return -errno;
+
+for (m = entry-conf; ; m = k + 1) {
+if (m[0] == '#')
+continue;
+
+k = strchr(m, '\n');
+
+if (k)
+*k = '\0';
+else
+break;
+
+if ((l = startswith(m, title )))
+entry-title = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, version )))
+entry-version= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, machine-id )))
+(void)sd_id128_from_string(l + strspn(l,
WHITESPACE), entry-machine_id);
+else if ((l = startswith(m, options )))
+entry-options= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, linux )))
+entry-linux_loc = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, initrd )))
+entry-initrd = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, efi )))
+entry-efi= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, devicetree )))
+entry-devicetree = l + strspn(l, WHITESPACE);
+else
+continue;
+}
+
+/* not interested in EFI programs */
+if (!entry-linux_loc) {
+i--;
+continue;
+}
+
+if (entry-node != rbtree_insert(entry-node, tree)) {
+/* already an entry with the same version string
[systemd-devel] [PATCH 4/8] update TODO
--- TODO | 2 ++ 1 file changed, 2 insertions(+) diff --git a/TODO b/TODO index 52a32d3..bf66ba1 100644 --- a/TODO +++ b/TODO @@ -32,6 +32,8 @@ External: * When lz4 gets an API for lz4 command output, make use of it to compress coredumps in a way compatible with /usr/bin/lz4. +* Fix emacs for Lennart so we can get rid of the Makefile hack littering git + Features: * journalctl --verify: don't show files that are currently being -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 7/8] add rpmvercmp()
---
Makefile.am| 2 +
src/shared/rpmvercmp.c | 122 +
src/shared/rpmvercmp.h | 14 ++
3 files changed, 138 insertions(+)
create mode 100644 src/shared/rpmvercmp.c
create mode 100644 src/shared/rpmvercmp.h
diff --git a/Makefile.am b/Makefile.am
index e0813a6..1310a20 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -900,6 +900,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/nss-util.h \
src/shared/verbs.c \
src/shared/verbs.h \
+ src/shared/rpmvercmp.c \
+ src/shared/rpmvercmp.h \
src/shared/sigbus.c \
src/shared/sigbus.h \
src/shared/rbtree.c \
diff --git a/src/shared/rpmvercmp.c b/src/shared/rpmvercmp.c
new file mode 100644
index 000..c69c2e3
--- /dev/null
+++ b/src/shared/rpmvercmp.c
@@ -0,0 +1,122 @@
+/* From rpm (Library GPLv2+, which is compatible with LGPLv2.1+)
+ * git://rpm.org/rpm.git
+ */
+
+#include stddef.h
+#include ctype.h
+#include string.h
+
+#include util.h
+#include rpmvercmp.h
+
+/* compare alpha and numeric segments of two versions */
+/* return 1: a is newer than b */
+/*0: a and b are the same version */
+/* -1: b is newer than a */
+int rpmvercmp(const char * a, const char * b)
+{
+/* easy comparison to see if versions are identical */
+if (streq_ptr(a, b)) return 0;
+
+char oldch1, oldch2;
+char abuf[strlen(a)+1], bbuf[strlen(b)+1];
+char *str1 = abuf, *str2 = bbuf;
+char * one, * two;
+int rc;
+int isnum;
+
+strcpy(str1, a);
+strcpy(str2, b);
+
+one = str1;
+two = str2;
+
+/* loop through each version segment of str1 and str2 and compare them */
+while (*one || *two) {
+ while (*one !isalnum(*one) *one != '~') one++;
+ while (*two !isalnum(*two) *two != '~') two++;
+
+ /* handle the tilde separator, it sorts before everything else */
+ if (*one == '~' || *two == '~') {
+ if (*one != '~') return 1;
+ if (*two != '~') return -1;
+ one++;
+ two++;
+ continue;
+ }
+
+ /* If we ran to the end of either, we are finished with the loop */
+ if (!(*one *two)) break;
+
+ str1 = one;
+ str2 = two;
+
+ /* grab first completely alpha or completely numeric segment */
+ /* leave one and two pointing to the start of the alpha or numeric */
+ /* segment and walk str1 and str2 to end of segment */
+ if (isdigit(*str1)) {
+ while (*str1 isdigit(*str1)) str1++;
+ while (*str2 isdigit(*str2)) str2++;
+ isnum = 1;
+ } else {
+ while (*str1 isalpha(*str1)) str1++;
+ while (*str2 isalpha(*str2)) str2++;
+ isnum = 0;
+ }
+
+ /* save character at the end of the alpha or numeric segment */
+ /* so that they can be restored after the comparison */
+ oldch1 = *str1;
+ *str1 = '\0';
+ oldch2 = *str2;
+ *str2 = '\0';
+
+ /* this cannot happen, as we previously tested to make sure that */
+ /* the first string has a non-null segment */
+ if (one == str1) return -1; /* arbitrary */
+
+ /* take care of the case where the two version segments are */
+ /* different types: one numeric, the other alpha (i.e. empty) */
+ /* numeric segments are always newer than alpha segments */
+ /* XXX See patch #60884 (and details) from bugzilla #50977. */
+ if (two == str2) return (isnum ? 1 : -1);
+
+ if (isnum) {
+ size_t onelen, twolen;
+ /* this used to be done by converting the digit segments */
+ /* to ints using atoi() - it's changed because long */
+ /* digit segments can overflow an int - this should fix that. */
+
+ /* throw away any leading zeros - it's a number, right? */
+ while (*one == '0') one++;
+ while (*two == '0') two++;
+
+ /* whichever number has more digits wins */
+ onelen = strlen(one);
+ twolen = strlen(two);
+ if (onelen twolen) return 1;
+ if (twolen onelen) return -1;
+ }
+
+ /* strcmp will return which one is greater - even if the two */
+ /* segments are alpha or if they are numeric. don't return */
+ /* if they are equal because there might be more segments to */
+ /* compare */
+ rc = strcmp(one, two);
+ if (rc) return (rc 1 ? -1 : 1);
+
+ /* restore character that was replaced by null above */
+ *str1 = oldch1;
+ one = str1;
+ *str2 = oldch2;
+ two = str2;
+}
+
+/* this catches the case where all numeric and alpha segments have */
+/* compared identically but the segment sepparating characters were */
+/* different */
+if ((!*one) (!*two)) return 0;
+
+/* whichever version still has characters left over wins */
+if (!*one) return -1; else return 1;
+}
diff --git
[systemd-devel] [PATCH 1/8] man: these binaries are internal APIs
--- man/systemd-halt.service.xml | 1 - man/systemd-shutdownd.service.xml | 1 - man/systemd-suspend.service.xml | 1 - 3 files changed, 3 deletions(-) diff --git a/man/systemd-halt.service.xml b/man/systemd-halt.service.xml index c94e2a1..7e7f8f2 100644 --- a/man/systemd-halt.service.xml +++ b/man/systemd-halt.service.xml @@ -56,7 +56,6 @@ parafilenamesystemd-poweroff.service/filename/para parafilenamesystemd-reboot.service/filename/para parafilenamesystemd-kexec.service/filename/para -parafilename/usr/lib/systemd/systemd-shutdown/filename/para /refsynopsisdiv refsect1 diff --git a/man/systemd-shutdownd.service.xml b/man/systemd-shutdownd.service.xml index 756949c..051d2ab 100644 --- a/man/systemd-shutdownd.service.xml +++ b/man/systemd-shutdownd.service.xml @@ -52,7 +52,6 @@ refsynopsisdiv parafilenamesystemd-shutdownd.service/filename/para parafilenamesystemd-shutdownd.socket/filename/para -parafilename/usr/lib/systemd/systemd-shutdownd/filename/para /refsynopsisdiv refsect1 diff --git a/man/systemd-suspend.service.xml b/man/systemd-suspend.service.xml index a8beb86..8e3df5f 100644 --- a/man/systemd-suspend.service.xml +++ b/man/systemd-suspend.service.xml @@ -56,7 +56,6 @@ parafilenamesystemd-suspend.service/filename/para parafilenamesystemd-hibernate.service/filename/para parafilenamesystemd-hybrid-sleep.service/filename/para -parafilename/usr/lib/systemd/system-sleep/filename/para /refsynopsisdiv refsect1 -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/8] man/systemd.timer.xml: improve documentation of WakeSystem=
--- man/systemd.timer.xml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/man/systemd.timer.xml b/man/systemd.timer.xml index 20890f2..4207be0 100644 --- a/man/systemd.timer.xml +++ b/man/systemd.timer.xml @@ -230,8 +230,9 @@ be suspended and if the system supports this. Note that this option will only make sure the system resumes on the appropriate times, it will not take care of suspending it -again after any work that is to be done is finished. Defaults -to varnamefalse/varname./para/listitem +again after any work that is to be done is finished. (For that +use varnameExecStartPost=systemd-suspend.service/varname in the triggered Unit=) +Defaults to varnamefalse/varname./para/listitem /varlistentry /variablelist /refsect1 -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 5/8] power: these binaries are internal APIs
They are not executed by a user (they all check how they were executed)
so we can use assert() in main() just like we would anywhere else.
---
src/power/shutdown.c | 20 ++--
src/power/shutdownd.c | 22 --
src/power/sleep.c | 14 +++---
3 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/src/power/shutdown.c b/src/power/shutdown.c
index 71f001a..ffd12b8 100644
--- a/src/power/shutdown.c
+++ b/src/power/shutdown.c
@@ -162,24 +162,24 @@ int main(int argc, char *argv[]) {
int cmd, r;
static const char* const dirs[] = {SYSTEM_SHUTDOWN_PATH, NULL};
-log_parse_environment();
-r = parse_argv(argc, argv);
-if (r 0)
-goto error;
+/* we are executed by execve() (without fork())
+ * pid 1's main() at the bottom of src/core/main.c
+ */
+assert(getpid() == 1);
+log_set_target(LOG_TARGET_AUTO);
+log_parse_environment();
/* journald will die if not gone yet. The log target defaults
* to console, but may have been changed by command line options. */
-
log_close_console(); /* force reopen of /dev/console */
log_open();
+r = parse_argv(argc, argv);
+assert(r = 0);
+
umask(0022);
-if (getpid() != 1) {
-log_error(Not executed by init (PID 1).);
-r = -EPERM;
-goto error;
-}
+in_container = detect_container(NULL) 0;
if (streq(arg_verb, reboot))
cmd = RB_AUTOBOOT;
diff --git a/src/power/shutdownd.c b/src/power/shutdownd.c
index 60a6468..742e1d5 100644
--- a/src/power/shutdownd.c
+++ b/src/power/shutdownd.c
@@ -264,15 +264,9 @@ int main(int argc, char *argv[]) {
bool exec_shutdown = false, unlink_nologin = false;
unsigned i;
-if (getppid() != 1) {
-log_error(This program should be invoked by init only.);
-return EXIT_FAILURE;
-}
-
-if (argc 1) {
-log_error(This program does not take arguments.);
-return EXIT_FAILURE;
-}
+/* we are executed through
systemd-shutdownd.socket-systemd-shutdownd.service */
+assert(getppid() == 1);
+assert(argc == 0);
log_set_target(LOG_TARGET_AUTO);
log_parse_environment();
@@ -281,15 +275,7 @@ int main(int argc, char *argv[]) {
umask(0022);
n_fds = sd_listen_fds(true);
-if (n_fds 0) {
-log_error_errno(r, Failed to read listening file descriptors
from environment: %m);
-return EXIT_FAILURE;
-}
-
-if (n_fds != 1) {
-log_error(Need exactly one file descriptor.);
-return EXIT_FAILURE;
-}
+assert(n_fds == 1);
pollfd[FD_SOCKET].fd = SD_LISTEN_FDS_START;
pollfd[FD_SOCKET].events = POLLIN;
diff --git a/src/power/sleep.c b/src/power/sleep.c
index cc1ffa6..2462082 100644
--- a/src/power/sleep.c
+++ b/src/power/sleep.c
@@ -200,17 +200,25 @@ int main(int argc, char *argv[]) {
_cleanup_strv_free_ char **modes = NULL, **states = NULL;
int r;
+/* we are executed through
+ * - systemd-suspend.service
+ * - systemd-hibernate.service
+ * - systemd-hybrid-sleep.service
+ */
+assert(getppid() == 1);
+
log_set_target(LOG_TARGET_AUTO);
log_parse_environment();
log_open();
r = parse_argv(argc, argv);
-if (r = 0)
-goto finish;
+assert(r 0);
r = parse_sleep_config(arg_verb, modes, states);
-if (r 0)
+if (r 0) {
+log_error_errno(r, Failed to parse PKGSYSCONFDIR
/sleep.conf: %m);
goto finish;
+}
r = execute(modes, states);
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [v1] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily
with kexec.);
-log_info(Rebooting with kexec.);
+/* kexec-tools has a bunch of special code to make Xen Dom0
work,
+ * otherwise it is only doing stuff we have already done.
+ * This is true for Dom0 and DomU but we only get Dom0
+ * because of the !in_container check */
+if (access(/proc/xen, F_OK) == 0) {
+pid_t pid;
pid = fork();
if (pid 0)
@@ -370,7 +376,9 @@ int main(int argc, char *argv[]) {
_exit(EXIT_FAILURE);
} else
wait_for_terminate_and_warn(kexec, pid,
true);
-}
+
+} else
+reboot(cmd);
cmd = RB_AUTOBOOT;
/* Fall through */
diff --git a/src/shared/missing.h b/src/shared/missing.h
index b33a70c..d80b2a7 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -35,6 +35,7 @@
#include linux/loop.h
#include linux/audit.h
#include linux/capability.h
+#include linux/kexec.h
#ifdef HAVE_AUDIT
#include libaudit.h
@@ -763,3 +764,13 @@ static inline int kcmp(pid_t pid1, pid_t pid2, int type,
unsigned long idx1, uns
#ifndef KCMP_FILE
#define KCMP_FILE 0
#endif
+
+/* v3.17 */
+#ifndef __NR_kexec_file_load
+#ifdef __x86_64__
+#define __NR_kexec_file_load 320
+#endif
+#endif
+#ifndef KEXEC_FILE_NO_INITRAMFS
+#define KEXEC_FILE_NO_INITRAMFS0x0004
+#endif
diff --git a/src/shared/rpmvercmp.c b/src/shared/rpmvercmp.c
index c69c2e3..09cfd97 100644
--- a/src/shared/rpmvercmp.c
+++ b/src/shared/rpmvercmp.c
@@ -13,8 +13,16 @@
/* return 1: a is newer than b */
/*0: a and b are the same version */
/* -1: b is newer than a */
-int rpmvercmp(const char * a, const char * b)
-{
+int rpmvercmp(const char * a, const char * b) {
+if (!a) {
+if (b)
+return -1;
+else
+return 0;
+}
+if (!b)
+return 1;
+
/* easy comparison to see if versions are identical */
if (streq_ptr(a, b)) return 0;
diff --git a/src/shared/strv.c b/src/shared/strv.c
index e27ac68..d983665 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -82,7 +82,14 @@ void strv_clear(char **l) {
}
void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
free(l);
}
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..0a43c7b
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,204 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Shawn Landden
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include ctype.h
+#include dirent.h
+#include sys/utsname.h
+#include linux/kexec.h
+
+#include bootspec.h
+#include strv.h
+#include fileio.h
+#include rpmvercmp.h
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s-conf);
+free(s);
+}
+
+static int bootspec_cmp(const struct rbtree_node *left, const struct
rbtree_node *right) {
+struct BootSpec *l = rbtree_container_of( left, struct BootSpec, node),
+*r = rbtree_container_of(right, struct BootSpec, node);
+
+return rpmvercmp(l-version, r-version);
+}
+
+int kernel_bootloaderspec_readconf(struct rbtree *tree) {
+int r = 0;
+_cleanup_closedir_ DIR *entries;
+struct dirent *dir;
+
+rbtree_init(tree, bootspec_cmp, 0);
+
+entries = opendir(BOOTENTRIESDIR);
+if (!entries) {
+if (r == -ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (size_t i=0;(dir = readdir(entries));i++) {
+struct BootSpec *entry;
+/* compiler wont allow 256 + strlen(BOOTENTRIESDIR) here */
+char fn[512] = {BOOTENTRIESDIR}, *d_name =
fn[strlen(BOOTENTRIESDIR
[systemd-devel] [PATCH] fix build against v3.20-rc1
--- fs.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs.c b/fs.c index 22ca62b..f751392 100644 --- a/fs.c +++ b/fs.c @@ -208,7 +208,6 @@ static struct inode *fs_inode_get(struct super_block *sb, inode-i_private = kdbus_node_ref(node); inode-i_mapping-a_ops = empty_aops; - inode-i_mapping-backing_dev_info = noop_backing_dev_info; inode-i_mode = node-mode S_IALLUGO; inode-i_atime = inode-i_ctime = inode-i_mtime = CURRENT_TIME; inode-i_uid = node-uid; -- 2.2.1.209.g41e5f3a ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily
The bootloader spec does not say which entry is to be the default. I cannot
support the spec unless I can choose a single default kernel.
http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
On Tue, Feb 17, 2015 at 7:00 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Mon, Feb 16, 2015 at 07:53:47PM -0800, Shawn Landden wrote:
On Mon, Feb 16, 2015 at 5:08 AM, Lennart Poettering
lenn...@poettering.net
wrote:
On Fri, 13.02.15 14:18, Shawn Landden (sh...@churchofgit.com) wrote:
Still use helper when Xen Dom0, to avoid duplicating some hairy
code.
Hmm, what precisely does the helper do on xen?
So we don't have any logic to load kexec kernels?
Currently we don't.
My hope though was that we can make the whole kexec thing work without
having kexec-tools installed. With the new kernel syscall for loading
the kernel we should be able to implement this all without any other
tools.
Ideally we'd not use any external tools anymore, not even in the Xen
case, hence I am curious what precisely the special hookup for Xen is
here...?
Lennart
https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c
I've attached my patch.
I'm having a problem with kexec_file_load() returning ENOMEM that I
havn't
resolved.
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
From d5446e324143f55e67bc2defe1c78a4ea4201142 Mon Sep 17 00:00:00 2001
From: Shawn Landden sh...@churchofgit.com
Date: Fri, 13 Feb 2015 13:48:07 -0800
Subject: [PATCH] shutdown: avoid calling `kexec` binary unnessecarily
Still use helper when Xen Dom0, to avoid duplicating some hairy code.
So we don't have any logic to load kexec kernels?
---
TODO | 3 --
src/core/shutdown.c | 121
++-
src/shared/missing.h | 7 +++
3 files changed, 116 insertions(+), 15 deletions(-)
diff --git a/TODO b/TODO
index 255a4f2..d914d2c 100644
--- a/TODO
+++ b/TODO
@@ -90,9 +90,6 @@ Features:
* maybe introduce WantsMountsFor=? Usecase:
http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
-* rework kexec logic to use new kexec_file_load() syscall, so that we
- don't have to call kexec tool anymore.
-
* The udev blkid built-in should expose a property that reflects
whether media was sensed in USB CF/SD card readers. This should then
be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 71f001a..14febf3 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -19,6 +19,7 @@
along with systemd; If not, see http://www.gnu.org/licenses/.
***/
+#include ctype.h
#include sys/mman.h
#include sys/types.h
#include sys/reboot.h
@@ -27,6 +28,7 @@
#include sys/stat.h
#include sys/mount.h
#include sys/syscall.h
+#include sys/utsname.h
#include fcntl.h
#include dirent.h
#include errno.h
@@ -138,6 +140,35 @@ static int parse_argv(int argc, char *argv[]) {
return 0;
}
+/*
+ * Remove parameter from a kernel command line. Helper function for
kexec.
+ * (copied from kexec-tools)
+ */
+static void remove_parameter(char *line, const char *param_name) {
+char *start, *end;
+
+start = strstr(line, param_name);
+
+/* parameter not found */
+if (!start)
+return;
+
+/*
+ * check if that's really the start of a parameter and not in
+ * the middle of the word
+ */
+if (start != line !isspace(*(start-1)))
+return;
+
+end = strstr(start, );
+if (!end)
+*start = 0;
+else {
+memmove(start, end+1, strlen(end));
+*(end + strlen(end)) = 0;
+}
+}
+
static int switch_root_initramfs(void) {
if (mount(/run/initramfs, /run/initramfs, NULL, MS_BIND,
NULL) 0)
return log_error_errno(errno, Failed to mount bind
/run/initramfs on /run/initramfs: %m);
@@ -152,6 +183,57 @@ static int switch_root_initramfs(void) {
return switch_root(/run/initramfs, /oldroot, false,
MS_BIND);
}
+static int kernel_load(bool overwrite) {
+int r = -ENOTSUP;
+
+/* only x86_64 and x32 in 3.18 */
+#ifdef __NR_kexec_file_load
+/* If uses has no already loaded a kexec kernel assume
they
+ * want the same one they are currently running. */
+if (!overwrite !kexec_loaded()) {
+struct utsname u;
+_cleanup_free_
Re: [systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily
On Tue, Feb 17, 2015 at 7:00 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
On Mon, Feb 16, 2015 at 07:53:47PM -0800, Shawn Landden wrote:
On Mon, Feb 16, 2015 at 5:08 AM, Lennart Poettering
lenn...@poettering.net
wrote:
On Fri, 13.02.15 14:18, Shawn Landden (sh...@churchofgit.com) wrote:
Still use helper when Xen Dom0, to avoid duplicating some hairy
code.
Hmm, what precisely does the helper do on xen?
So we don't have any logic to load kexec kernels?
Currently we don't.
My hope though was that we can make the whole kexec thing work without
having kexec-tools installed. With the new kernel syscall for loading
the kernel we should be able to implement this all without any other
tools.
Ideally we'd not use any external tools anymore, not even in the Xen
case, hence I am curious what precisely the special hookup for Xen is
here...?
Lennart
https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c
I've attached my patch.
I'm having a problem with kexec_file_load() returning ENOMEM that I
havn't
resolved.
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Shawn Landden
From d5446e324143f55e67bc2defe1c78a4ea4201142 Mon Sep 17 00:00:00 2001
From: Shawn Landden sh...@churchofgit.com
Date: Fri, 13 Feb 2015 13:48:07 -0800
Subject: [PATCH] shutdown: avoid calling `kexec` binary unnessecarily
Still use helper when Xen Dom0, to avoid duplicating some hairy code.
So we don't have any logic to load kexec kernels?
---
TODO | 3 --
src/core/shutdown.c | 121
++-
src/shared/missing.h | 7 +++
3 files changed, 116 insertions(+), 15 deletions(-)
diff --git a/TODO b/TODO
index 255a4f2..d914d2c 100644
--- a/TODO
+++ b/TODO
@@ -90,9 +90,6 @@ Features:
* maybe introduce WantsMountsFor=? Usecase:
http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
-* rework kexec logic to use new kexec_file_load() syscall, so that we
- don't have to call kexec tool anymore.
-
* The udev blkid built-in should expose a property that reflects
whether media was sensed in USB CF/SD card readers. This should then
be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 71f001a..14febf3 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -19,6 +19,7 @@
along with systemd; If not, see http://www.gnu.org/licenses/.
***/
+#include ctype.h
#include sys/mman.h
#include sys/types.h
#include sys/reboot.h
@@ -27,6 +28,7 @@
#include sys/stat.h
#include sys/mount.h
#include sys/syscall.h
+#include sys/utsname.h
#include fcntl.h
#include dirent.h
#include errno.h
@@ -138,6 +140,35 @@ static int parse_argv(int argc, char *argv[]) {
return 0;
}
+/*
+ * Remove parameter from a kernel command line. Helper function for
kexec.
+ * (copied from kexec-tools)
+ */
+static void remove_parameter(char *line, const char *param_name) {
+char *start, *end;
+
+start = strstr(line, param_name);
+
+/* parameter not found */
+if (!start)
+return;
+
+/*
+ * check if that's really the start of a parameter and not in
+ * the middle of the word
+ */
+if (start != line !isspace(*(start-1)))
+return;
+
+end = strstr(start, );
+if (!end)
+*start = 0;
+else {
+memmove(start, end+1, strlen(end));
+*(end + strlen(end)) = 0;
+}
+}
+
static int switch_root_initramfs(void) {
if (mount(/run/initramfs, /run/initramfs, NULL, MS_BIND,
NULL) 0)
return log_error_errno(errno, Failed to mount bind
/run/initramfs on /run/initramfs: %m);
@@ -152,6 +183,57 @@ static int switch_root_initramfs(void) {
return switch_root(/run/initramfs, /oldroot, false,
MS_BIND);
}
+static int kernel_load(bool overwrite) {
+int r = -ENOTSUP;
+
+/* only x86_64 and x32 in 3.18 */
+#ifdef __NR_kexec_file_load
+/* If uses has no already loaded a kexec kernel assume
they
+ * want the same one they are currently running. */
+if (!overwrite !kexec_loaded()) {
+struct utsname u;
+_cleanup_free_ char *vmlinuz = NULL, *initrd =
NULL, *cmdline = NULL;
+_cleanup_close_ int vmlinuz_fd = -1, initrd_fd
= -1;
+
+r = uname(u
