<span class="">A few things:<br><ul><li>Please fix the spelling of "present."<br></li><li>The number -128, if used, should get #define'd instead of being a magic number dropped in a bunch of places.<br></li><li>
It's not good to mask all errors opening the machine-id with the -128 response code. The change here seems to make all opening errors return -128.</li></ul></span>More importantly, what's wrong with looking for the normal [Errno 2] "No such file or directory" return code?<div class="gmail_extra">
<br><br><div class="gmail_quote">On Fri, Nov 30, 2012 at 6:52 AM, Ramkumar Ramachandra <span dir="ltr"><<a href="mailto:artagnon@gmail.com" target="_blank">artagnon@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">The following tests fail if /etc/machine-id is not present:<br>
<br>
$ ./test-id128<br>
random: a08ea8ed34594d4bbd953dd182ec86f9<br>
Assertion 'sd_id128_get_machine(&id) == 0' failed at<br>
src/test/test-id128.c:41, function main(). Aborting.<br>
[1] 8017 abort (core dumped) ./test-id128<br>
<br>
$ ./test-journal<br>
Assertion 'journal_file_open("test.journal", O_RDWR|O_CREAT, 0666,<br>
true, true, NULL, NULL, NULL, &f) == 0' failed at<br>
src/journal/test-journal.c:46, function main(). Aborting.<br>
[1] 8059 abort (core dumped) ./test-journal<br>
<br>
$ ./test-journal-stream<br>
Assertion 'journal_file_open("one.journal", O_RDWR|O_CREAT, 0666,<br>
true, false, NULL, NULL, NULL, &one) == 0' failed at<br>
src/journal/test-journal-stream.c:88, function main(). Aborting.<br>
[1] 8107 abort (core dumped) ./test-journal-stream<br>
<br>
$ ./test-journal-verify<br>
Generating...<br>
Assertion 'journal_file_open("test.journal", O_RDWR|O_CREAT, 0666,<br>
true, !!verification_key, NULL, NULL, NULL, &f) == 0' failed at<br>
src/journal/test-journal-verify.c:87, function main(). Aborting.<br>
[1] 8154 abort (core dumped) ./test-journal-verify<br>
<br>
This is because they call sd_id128_get_machine() which barfs if<br>
/etc/machine-id can't be open()'ed. Treat this as a special case and<br>
skip the dependent tests instead of failing them.<br>
---<br>
</div></div> Oops; forgot to declate `id' in the previous iteration.<br>
<br>
src/journal/test-journal-stream.c | 6 ++++++<br>
src/journal/test-journal-verify.c | 6 ++++++<br>
src/journal/test-journal.c | 6 ++++++<br>
<div class="im"> src/libsystemd-id128/sd-id128.c | 2 +-<br>
src/test/test-id128.c | 8 ++++++--<br>
</div> 5 files changed, 25 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/src/journal/test-journal-stream.c b/src/journal/test-journal-stream.c<br>
index b3e816d..d07448f 100644<br>
--- a/src/journal/test-journal-stream.c<br>
+++ b/src/journal/test-journal-stream.c<br>
@@ -79,6 +79,12 @@ int main(int argc, char *argv[]) {<br>
char *z;<br>
<div class="im"> const void *data;<br>
size_t l;<br>
</div>+ sd_id128_t id;<br>
<div class="im">+<br>
+ if (sd_id128_get_machine(&id) == -128) {<br>
+ printf("skipping test: /etc/machine-id not preset\n");<br>
+ return 0;<br>
+ }<br>
<br>
</div> log_set_max_level(LOG_DEBUG);<br>
<br>
diff --git a/src/journal/test-journal-verify.c b/src/journal/test-journal-verify.c<br>
index b667721..ee55485 100644<br>
--- a/src/journal/test-journal-verify.c<br>
+++ b/src/journal/test-journal-verify.c<br>
@@ -76,6 +76,12 @@ int main(int argc, char *argv[]) {<br>
char c[FORMAT_TIMESPAN_MAX];<br>
<div class="im"> struct stat st;<br>
uint64_t p;<br>
</div>+ sd_id128_t id;<br>
<div class="im">+<br>
+ if (sd_id128_get_machine(&id) == -128) {<br>
+ printf("skipping test: /etc/machine-id not preset\n");<br>
+ return 0;<br>
+ }<br>
<br>
</div> log_set_max_level(LOG_DEBUG);<br>
<br>
diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c<br>
index f4dc52c..192b787 100644<br>
--- a/src/journal/test-journal.c<br>
+++ b/src/journal/test-journal.c<br>
@@ -37,6 +37,12 @@ int main(int argc, char *argv[]) {<br>
Object *o;<br>
<div class="im"> uint64_t p;<br>
char t[] = "/tmp/journal-XXXXXX";<br>
</div>+ sd_id128_t id;<br>
<div class="im">+<br>
+ if (sd_id128_get_machine(&id) == -128) {<br>
+ printf("skipping test: /etc/machine-id not preset\n");<br>
+ return 0;<br>
+ }<br>
<br>
</div> log_set_max_level(LOG_DEBUG);<br>
<div class="HOEnZb"><div class="h5"><br>
diff --git a/src/libsystemd-id128/sd-id128.c b/src/libsystemd-id128/sd-id128.c<br>
index 4286ae7..4e5aaad 100644<br>
--- a/src/libsystemd-id128/sd-id128.c<br>
+++ b/src/libsystemd-id128/sd-id128.c<br>
@@ -106,7 +106,7 @@ _public_ int sd_id128_get_machine(sd_id128_t *ret) {<br>
<br>
fd = open("/etc/machine-id", O_RDONLY|O_CLOEXEC|O_NOCTTY);<br>
if (fd < 0)<br>
- return -errno;<br>
+ return -128; /* Special return value */<br>
<br>
k = loop_read(fd, buf, 32, false);<br>
close_nointr_nofail(fd);<br>
diff --git a/src/test/test-id128.c b/src/test/test-id128.c<br>
index bfd743e..ea621f9 100644<br>
--- a/src/test/test-id128.c<br>
+++ b/src/test/test-id128.c<br>
@@ -38,8 +38,12 @@ int main(int argc, char *argv[]) {<br>
assert_se(sd_id128_from_string(t, &id2) == 0);<br>
assert_se(sd_id128_equal(id, id2));<br>
<br>
- assert_se(sd_id128_get_machine(&id) == 0);<br>
- printf("machine: %s\n", sd_id128_to_string(id, t));<br>
+ if (sd_id128_get_machine(&id) == -128)<br>
+ printf("skipping test: /etc/machine-id not preset\n");<br>
+ else {<br>
+ assert_se(sd_id128_get_machine(&id) == 0);<br>
+ printf("machine: %s\n", sd_id128_to_string(id, t));<br>
+ }<br>
<br>
assert_se(sd_id128_get_boot(&id) == 0);<br>
printf("boot: %s\n", sd_id128_to_string(id, t));<br>
--<br>
1.7.8.1.362.g5d6df.dirty<br>
<br>
_______________________________________________<br>
systemd-devel mailing list<br>
<a href="mailto:systemd-devel@lists.freedesktop.org">systemd-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/systemd-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/systemd-devel</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><table cellspacing="0" cellpadding="0" width="100%"><tbody><tr><td><pre>David Strauss
| <a href="mailto:david@davidstrauss.net" target="_blank">david@davidstrauss.net</a>
| +1 512 577 5827 [mobile]
</pre>
</td>
</tr>
</tbody></table><br>
</div>