[systemd-devel] [PATCH] dbus1-generator: Always close file in create_dbus_files
In the !service case, the first file doesn't get closed automatically,
since the second one uses the same FILE*. Close it explicitly.
Found by cppcheck
Signed-off-by: Simon Danner danner.si...@gmail.com
---
src/dbus1-generator/dbus1-generator.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/dbus1-generator/dbus1-generator.c
b/src/dbus1-generator/dbus1-generator.c
index e1ffc55..e401471 100644
--- a/src/dbus1-generator/dbus1-generator.c
+++ b/src/dbus1-generator/dbus1-generator.c
@@ -100,8 +100,7 @@ static int create_dbus_files(
}
}
-fflush(f);
-if (ferror(f)) {
+if (fclose(f)) {
log_error(Failed to write %s: %m, a);
return -errno;
}
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] timesyncd: Frequent polling when no server could be reached
On Thu, Aug 21, 2014 at 08:15:36PM +0200, Florian Lindner wrote: Aug 21 09:45:00 asaru systemd-timesyncd[317]: Timed out waiting for reply from 216.239.32.15:123 (time1.google.com). Aug 21 09:45:00 asaru systemd-timesyncd[317]: Using NTP server [2001:4860:4802:32::f]:123 (time1.google.com). Aug 21 09:45:00 asaru systemd-timesyncd[317]: Using NTP server 216.239.34.15:123 (time2.google.com). Aug 21 09:45:10 asaru systemd-timesyncd[317]: Timed out waiting for reply from 216.239.34.15:123 (time2.google.com). Aug 21 09:45:10 asaru systemd-timesyncd[317]: Using NTP server [2001:4860:4802:34::f]:123 (time2.google.com). Aug 21 09:45:10 asaru systemd-timesyncd[317]: Using NTP server 216.239.36.15:123 (time3.google.com). Polluting my log this way. Is is possible to inhibit that behavior? Maybe trying a couple of times, then giving up until next network status change. Hm, a well behaved client reduces its polling rate exponentially when it doesn't receive a reply to avoid overloading the servers and network congestion. After running some tests, it seems there is an even bigger problem. When timesyncd receives a reply saying that the server isn't synchronized or that the client should reduce its polling rate (KOD RATE), it selects the next server and sends a new request immediately. When all servers are unsynchronized, this creates a burst of 10 packets several times per minute. This really needs to be fixed. An easy solution could be to add an exponentially increasing delay (with maximum at 2048 seconds) when all servers were tried and switching back to the first server in the list. Clients increasing their polling rate when not receiving reply or receiving a reply they don't like is the biggest problem the pool.ntp.org operators have to deal with. BTW, I was getting segfaults with current git in sd_resolve_getaddrinfo() in manager_connect() when doing the tests, removing the server_name_flush_addresses() call seems to fix it. Another question I have is about the NTP status output of timedatectl. Right now (with ntpd running) it says: NTP enabled: yes NTP synchronized: no I suppose it need some more uptime than the 11 minutes I have currently? Possibly, ntpd needs to clear the STA_UNSYNC flag in adjtimex to mark the clock as synchronized. When I had timesyncd active it said NTP no, though I had a ntp client runnung, albeit definitly unsynchronized. Version is 215 With 215, you need to remove all files in /usr/lib/systemd/ntp-units.d/ except the one that lists the timesyncd service to select it for timedated. In 216 the ntp-units.d directory is ignored and timedated always controls timesyncd. I think it would be nice if this was configurable at least at compile time and I sent a patch for that yesterday. -- Miroslav Lichvar ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] dbus1-generator: Always close file in create_dbus_files
On Fri, 22.08.14 08:19, Simon Danner (danner.si...@gmail.com) wrote:
In the !service case, the first file doesn't get closed automatically,
since the second one uses the same FILE*. Close it explicitly.
Found by cppcheck
Signed-off-by: Simon Danner danner.si...@gmail.com
---
src/dbus1-generator/dbus1-generator.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/dbus1-generator/dbus1-generator.c
b/src/dbus1-generator/dbus1-generator.c
index e1ffc55..e401471 100644
--- a/src/dbus1-generator/dbus1-generator.c
+++ b/src/dbus1-generator/dbus1-generator.c
@@ -100,8 +100,7 @@ static int create_dbus_files(
}
}
-fflush(f);
-if (ferror(f)) {
+if (fclose(f)) {
log_error(Failed to write %s: %m, a);
return -errno;
}
It's actually more complicated. We need to set f to NULL too, otherwise
the cleanup logic might invoke fclose() again, on an invalidated FILE*.
I have now commited a patch that reworks this to use fflush_and_check()
and then closes the thing and resets f to NULL.
Thanks for the pointer!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] man/journalctl.xml src/journal
On Fri, 22.08.14 01:57, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
On Thu, Aug 21, 2014 at 05:16:37AM -0700, Harald Hoyer wrote:
@@ -276,6 +278,7 @@ static int parse_argv(int argc, char *argv[]) {
{ file, required_argument, NULL, ARG_FILE
},
{ root, required_argument, NULL, ARG_ROOT
},
{ header, no_argument, NULL, ARG_HEADER
},
+{ identifier, required_argument, NULL, 't'
},
{ priority, required_argument, NULL, 'p'
},
{ setup-keys, no_argument, NULL,
ARG_SETUP_KEYS },
{ interval, required_argument, NULL, ARG_INTERVAL
},
@@ -304,7 +307,7 @@ static int parse_argv(int argc, char *argv[]) {
assert(argc = 0);
assert(argv);
-while ((c = getopt_long(argc, argv,
hefo:aln::qmb::kD:p:c:u:F:xrM:, options, NULL)) = 0)
+while ((c = getopt_long(argc, argv,
hefo:aln::qmb::kD:p:c:t:u:F:xrM:, options, NULL)) = 0)
Hi,
I think the addition is useful, but I'm not sure we want to immediately
allocate
the short option too. So far syslog identifier haven't been in much use, and
't' is a nice option name.
I think Harald has a point in that this would neatly mirror what
systemd-cat already supports... I'd vote for leaving it in.
I wasn't convinced by the patch at first, since it brings very little
new in comparison to simply specifying SYSLOG_IDENTIFIER=
instead. However, there are two reasons I ended up liking it: first,
because it does mirror systemd-cat nicely, but, more importantly, it's
actually a useful key to sort by that is shown in the usual journalctl
output, which none of the other usual keys are. This means that people
can look at the normal output, and immediately deduce a key to filter
by, something that for the other keys (includeing _SYSTEMD_UNIT=...)
requires them to invoke journalctl -o verbose first... Hence I think
it deserves being very easily accessible, including by short option...
I hope that makes some sense,
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Sat, Aug 16, 2014 at 1:29 PM, Daniele Nicolodi dani...@grinta.net wrote:
On 16/08/2014 12:35, David Herrmann wrote:
On Fri, Aug 15, 2014 at 5:22 PM, Daniele Nicolodi dani...@grinta.net wrote:
this may be completely stupid, but if the only use case you have for
CONST_MAX() is for computing the size of a data structure, I find
something like
#define MAXSIZE(A, B) sizeof(union { __typeof(A) a; __typeof(B) b;})
a little more clear and less magic, and I believe it has the same
guarantees that the solution you found.
Your MAXSIZE macro might add padding:
This union has size 8, not 5 (64bit). But CONST_MAX would return 5.
Not sure whether that really matters, though. And we could probably
add __packed__ to the definition.
Indeed it does add padding. Adding the __packed__ attribute solves the
problem:
#define MAXSIZE(A, B) sizeof( \
union __attribute__((__packed__)) { \
__typeof(A) a; __typeof(B) b;})
However, I noticed that GCC complains about using
statement-expressions to initialize static-const structure members,
even with my 'const' annotations added to MAX. *sigh*
Thus, I think I'll keep CONST_MAX, as we'd require a 3rd macro otherwise.
My proposal was based on the fact that the only use of CONST_MAX there
is (was?) in the code was about array size declarations, and I find
MAXSIZE() much easier to understand.
I've added the macro now.
Thanks!
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] shared: make container_of() use unique variable names
If you stack container_of() macros, you will get warnings due to shadowing
variables of the parent context. To avoid this, use unique names for
variables.
Two new helpers are added:
UNIQ: This evaluates to a truly unique value never returned by any
evaluation of this macro. It's a shortcut for __COUNTER__.
UNIQ_T: Takes two arguments and concatenates them. It is a shortcut for
CONCATENATE, but meant to defined typed local variables.
As you usually want to use variables that you just defined, you need to
reference the same unique value at least two times. However, UNIQ returns
a new value on each evaluation, therefore, you have to pass the unique
values into the macro like this:
#define my_macro(a, b) __max_macro(UNIQ, UNIQ, (a), (b))
#define __my_macro(uniqa, uniqb, a, b) ({
typeof(a) UNIQ_T(A, uniqa) = (a);
typeof(b) UNIQ_T(B, uniqb) = (b);
MY_UNSAFE_MACRO(UNIQ_T(A, uniqa), UNIQ_T(B, uniqb));
})
This way, MY_UNSAFE_MACRO() can safely evaluate it's arguments multiple
times as they are local variables. But you can also stack invocations to
the macro my_macro() without clashing names.
This is the same as if you did:
#define my_macro(a, b) __max_macro(__COUNTER__, __COUNTER__, (a), (b))
#define __my_macro(prefixa, prefixb, a, b) ({
typeof(a) CONCATENATE(A, prefixa) = (a);
typeof(b) CONCATENATE(B, prefixb) = (b);
MY_UNSAFE_MACRO(CONCATENATE(A, prefixa), CONCATENATE(B,
prefixb));
})
...but in my opinion, the first macro is easier to write and read.
This patch starts by converting container_of() to use this new helper.
Other macros may follow (like MIN, MAX, CLAMP, ...).
---
src/shared/macro.h | 13 -
src/test/test-util.c | 19 +++
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 2807bc7..e673480 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -79,6 +79,9 @@
#define XCONCATENATE(x, y) x ## y
#define CONCATENATE(x, y) XCONCATENATE(x, y)
+#define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
+#define UNIQ __COUNTER__
+
/* Rounds up */
#define ALIGN4(l) (((l) + 3) ~3)
@@ -122,13 +125,13 @@ static inline unsigned long ALIGN_POWER2(unsigned long u)
{
* @ptr: the pointer to the member.
* @type: the type of the container struct this is embedded in.
* @member: the name of the member within the struct.
- *
*/
-#define container_of(ptr, type, member) \
+#define container_of(ptr, type, member) __container_of(UNIQ, (ptr), type,
member)
+#define __container_of(uniq, ptr, type, member) \
__extension__ ({\
-const typeof( ((type *)0)-member ) *__mptr = (ptr); \
-(type *)( (char *)__mptr - offsetof(type,member) ); \
-})
+const typeof( ((type*)0)-member ) *UNIQ_T(A, uniq) = (ptr); \
+(type*)( (char *)UNIQ_T(A, uniq) - offsetof(type,member) ); \
+})
#undef MAX
#define MAX(a,b)\
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 34d5f2e..7b2e71c 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -96,6 +96,24 @@ static void test_max(void) {
assert_cc(MAXSIZE(char, long) == sizeof(long));
}
+static void test_container_of(void) {
+struct mytype {
+uint8_t pad1[3];
+uint64_t v1;
+uint8_t pad2[2];
+uint32_t v2;
+} _packed_ myval = { };
+
+assert_cc(sizeof(myval) == 17);
+assert_se(container_of(myval.v1, struct mytype, v1) == myval);
+assert_se(container_of(myval.v2, struct mytype, v2) == myval);
+assert_se(container_of(container_of(myval.v2,
+ struct mytype,
+ v2)-v1,
+ struct mytype,
+ v1) == myval);
+}
+
static void test_first_word(void) {
assert_se(first_word(Hello, ));
assert_se(first_word(Hello, Hello));
@@ -1218,6 +1236,7 @@ int main(int argc, char *argv[]) {
test_streq_ptr();
test_align_power2();
test_max();
+test_container_of();
test_first_word();
test_close_many();
test_parse_boolean();
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] kdbus: merge 3.17 branch into master?
Hi, I am having __NR_memfd_create not defined error on mips on systemd 216. I wanted to see the syscall numbers in 3.17 RC1 to patch missing.h but couldn't find them. What am I missing? On Tue, Aug 19, 2014 at 4:24 PM, Daniel Mack dan...@zonque.org wrote: On 08/19/2014 04:21 PM, Greg KH wrote: On Tue, Aug 19, 2014 at 04:08:15PM +0200, Daniel Mack wrote: Could you please try if temporarily reverting my top-most commit and then doing a 'make headers_install' in your kernel repo (before you build kdbus) fixes it? Yes, that works. And we can live with that, it's reasonable to rely on that kernel header now. Alright. Revert commit is pushed now. Thanks for testing! Daniel ___ 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
Re: [systemd-devel] kdbus: merge 3.17 branch into master?
Hi, On 08/22/2014 03:26 PM, Umut Tezduyar Lindskog wrote: I am having __NR_memfd_create not defined error on mips on systemd 216. I wanted to see the syscall numbers in 3.17 RC1 to patch missing.h but couldn't find them. What am I missing? We don't know the final syscall number yet, as it's not yet hooked up for MIPS. For now, I've pushed a dummy value to missing.h now, which should fix your problem. Sorry for that. Note that until this is fixed in the kernel, you won't be able to use systemd with kdbus on MIPS. Just curious - did you ever try this? Thanks, Daniel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] kdbus: merge 3.17 branch into master?
Hi, On Fri, Aug 22, 2014 at 3:47 PM, Daniel Mack dan...@zonque.org wrote: Hi, On 08/22/2014 03:26 PM, Umut Tezduyar Lindskog wrote: I am having __NR_memfd_create not defined error on mips on systemd 216. I wanted to see the syscall numbers in 3.17 RC1 to patch missing.h but couldn't find them. What am I missing? We don't know the final syscall number yet, as it's not yet hooked up for MIPS. For now, I've pushed a dummy value to missing.h now, which should fix your problem. Sorry for that. No problem at all. Note that until this is fixed in the kernel, you won't be able to use systemd with kdbus on MIPS. Just curious - did you ever try this? Sorry, I haven't tested kdbus on mips. Umut Thanks, Daniel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] systemctl preset and chkconfig
Hi, I recently changed my %post scripts in Mageia to use systemctl preset rather than systemctl enable to allow for policy-based overrides of enable on install behaviour. Sadly, unlike enable, preset does not shell out to chkconfig, so passing a service name that's not got a native unit no longer gets enabled. Now I can work around this in our %post scripts, but an alternative would be to teach preset about chkconfig and shell out to that if a native unit is not found. I'm not overly bothered where I work around this and of course long term goal is not to ship any sysvinit scripts anyway. But before I work on a solution, would upstream be interested in preset supporting chkconfig? If not, it's probably quicker and easier for me to do the work and maintain it in scripts rather than systemctl itself, hence why I figured I'd ask first. Cheers Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] sd-bus: kdbus: monitor connections use the KDBUS_HELLO_MONITOR flag
---
Currently this bus_kernel_create_monitor() is not used.
Patch compile tested.
src/libsystemd/sd-bus/bus-kernel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libsystemd/sd-bus/bus-kernel.c
b/src/libsystemd/sd-bus/bus-kernel.c
index 3ca271c..1440e43 100644
--- a/src/libsystemd/sd-bus/bus-kernel.c
+++ b/src/libsystemd/sd-bus/bus-kernel.c
@@ -1547,7 +1547,7 @@ int bus_kernel_create_monitor(const char *bus) {
hello = alloca0(sizeof(struct kdbus_cmd_hello));
hello-size = sizeof(struct kdbus_cmd_hello);
-hello-conn_flags = KDBUS_HELLO_ACTIVATOR;
+hello-conn_flags = KDBUS_HELLO_MONITOR;
hello-pool_size = KDBUS_POOL_SIZE;
if (ioctl(fd, KDBUS_CMD_HELLO, hello) 0) {
--
1.9.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
