<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 17, 2016 at 7:49 AM, Dmitry V. Levin <span dir="ltr"><<a href="mailto:ldv@altlinux.org" target="_blank">ldv@altlinux.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Tue, Mar 15, 2016 at 05:06:48PM +0530, Jay Joshi wrote:<br>
> >From 5e543e3a0ae730b1833c8a4d5d9a5bf28273256d Mon Sep 17 00:00:00 2001<br>
> From: JayRJoshi <<a href="mailto:jay.r.joshi100@gmail.com">jay.r.joshi100@gmail.com</a>><br>
> Date: Tue, 15 Mar 2016 16:41:39 +0530<br>
> Subject: [PATCH] Test for getcwd (getcwd.c, getcwd.expected, getcwd.test)<br>
> added<br>
<br>
</span>This commit message doesn't follow our guidelines.<br>
It might be a good idea to run "git log -- tests" and see how commit<br>
messages for tests usually look like.<br>
<span class=""><br>
> --- /dev/null<br>
> +++ b/tests/getcwd.c<br>
> @@ -0,0 +1,36 @@<br>
> +#include "tests.h"<br>
> +<br>
> +#include <sys/stat.h><br>
> +#include <stdio.h><br>
> +#include <unistd.h><br>
> +#include <errno.h><br>
> +<br>
> +<br>
> +int<br>
> +main(void)<br>
> +{<br>
> + char cur_dir[32];<br>
> +<br>
> + if (mkdir("/tmp/sample_dir", S_IRWXU) == -1)<br>
> + perror_msg_and_skip("mkdir");<br>
<br>
</span>Why do you want to use /tmp in the first place?<br>
Please avoid /tmp if possible, it's unreliable.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> + if (chdir("/tmp/sample_dir") == -1)<br>
> + perror_msg_and_skip("chdir");<br>
> +<br>
> + if (getcwd(cur_dir, sizeof(cur_dir)) != NULL) {<br>
> + printf("getcwd(\"/tmp/sample_dir\", 32) = 16\n");<br>
<br>
</span>Now that you use /tmp, imagine that /tmp is a symlink to another<br>
directory. In that case getcwd would return something different from your<br>
expectations.<br></blockquote><div><br></div><div>Actually, I wanted to avoid using regexp for directory path. Also, /tmp was by default (and usually) is given all rwx permissions for all users. Although, I do agree with your point.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
> + } else {<br>
> + perror_msg_and_fail("getcwd");<br>
> + }<br>
> +<br>
> + if (rmdir(cur_dir) == -1)<br>
> + perror_msg_and_skip("rmdir");<br>
> +<br>
> + if (getcwd(cur_dir, sizeof(cur_dir)) != NULL) {<br>
> + printf("getcwd(\"/tmp/sample_dir\", 32) = 16\n");<br>
> + } else {<br>
> + printf("getcwd(\"0x0123456789ab, 32) = -1 ENOENT (%m)\n");<br>
<br>
</span>Why this magic number?<br>
<span class=""><br>
> --- /dev/null<br>
> +++ b/tests/getcwd.expected<br>
> @@ -0,0 +1,2 @@<br>
> +getcwd\(\"/tmp/sample_dir\", 32\) += 16<br>
> +getcwd\(0x[0-9a-f]+, 32\) += -1 ENOENT \(No such file or directory\)<br>
<br>
</span>Looks like you've got confused. If you have getcwd.expected, then<br>
getcwd.c doesn't have to print anything. If your getcwd.c prints expected<br>
output, then you don't need getcwd.expected. In other words, you need<br>
only one of two types of expected output, not both. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> diff --git a/tests/getcwd.test b/tests/getcwd.test<br>
> new file mode 100755<br>
> index 0000000..03fabb1<br>
> --- /dev/null<br>
> +++ b/tests/getcwd.test<br>
> @@ -0,0 +1,9 @@<br>
> +#!/bin/sh<br>
> +<br>
> +. "${srcdir=.}/init.sh"<br>
> +<br>
> +run_prog > /dev/null<br>
> +run_strace -egetcwd $args<br>
> +match_grep<br>
<br>
</span>Here your match_grep uses getcwd.expected, so the output produced by<br>
getcwd.c is ignored.<br>
<br>
I generally prefer match_diff to match_grep because the former is exact<br>
comparison and therefore the check is more robust.<br>
<span class=""><font color="#888888"><br></font></span></blockquote><div><br></div><div>I wanted to cover test for error condition ENOENT, I couldn't think of any way to check it using match_diff. Is it possible?</div><div>Updated patch is pasted below. I'm using regexp for directories. As, no nested regexp is used, I don't think performance will be concern. </div><div><br></div><div><div>From 27c04f259a56c2990511ab171ddbb283f0810b95 Mon Sep 17 00:00:00 2001</div><div>From: JayRJoshi <<a href="mailto:jay.r.joshi100@gmail.com">jay.r.joshi100@gmail.com</a>></div><div>Date: Tue, 15 Mar 2016 16:41:39 +0530</div><div>Subject: [PATCH] tests: add getcwd.test</div><div><br></div><div>---</div><div> tests/.gitignore | 1 +</div><div> tests/Makefile.am | 2 ++</div><div> tests/getcwd.c | 30 ++++++++++++++++++++++++++++++</div><div> tests/getcwd.expected | 2 ++</div><div> tests/getcwd.test | 9 +++++++++</div><div> 5 files changed, 44 insertions(+)</div><div> create mode 100644 tests/getcwd.c</div><div> create mode 100644 tests/getcwd.expected</div><div> create mode 100755 tests/getcwd.test</div><div><br></div><div>diff --git a/tests/.gitignore b/tests/.gitignore</div><div>index b89796a..52eb76d 100644</div><div>--- a/tests/.gitignore</div><div>+++ b/tests/.gitignore</div><div>@@ -41,6 +41,7 @@ fstat64</div><div> fstatat64</div><div> ftruncate</div><div> ftruncate64</div><div>+getcwd</div><div> getdents</div><div> getdents64</div><div> getrandom</div><div>diff --git a/tests/Makefile.am b/tests/Makefile.am</div><div>index 59e7e78..81ad778 100644</div><div>--- a/tests/Makefile.am</div><div>+++ b/tests/Makefile.am</div><div>@@ -91,6 +91,7 @@ check_PROGRAMS = \</div><div> <span class="" style="white-space:pre"> </span>fstatat64 \</div><div> <span class="" style="white-space:pre"> </span>ftruncate \</div><div> <span class="" style="white-space:pre"> </span>ftruncate64 \</div><div>+<span class="" style="white-space:pre"> </span>getcwd \</div><div> <span class="" style="white-space:pre"> </span>getdents \</div><div> <span class="" style="white-space:pre"> </span>getdents64 \</div><div> <span class="" style="white-space:pre"> </span>getrandom \</div><div>@@ -264,6 +265,7 @@ TESTS = \</div><div> <span class="" style="white-space:pre"> </span>fstatat64.test \</div><div> <span class="" style="white-space:pre"> </span>ftruncate.test \</div><div> <span class="" style="white-space:pre"> </span>ftruncate64.test \</div><div>+<span class="" style="white-space:pre"> </span>getcwd.test \</div><div> <span class="" style="white-space:pre"> </span>getdents.test \</div><div> <span class="" style="white-space:pre"> </span>getdents64.test \</div><div> <span class="" style="white-space:pre"> </span>getrandom.test \</div><div>diff --git a/tests/getcwd.c b/tests/getcwd.c</div><div>new file mode 100644</div><div>index 0000000..561a4f1</div><div>--- /dev/null</div><div>+++ b/tests/getcwd.c</div><div>@@ -0,0 +1,30 @@</div><div>+#include "tests.h"</div><div>+</div><div>+#include <sys/stat.h></div><div>+#include <stdio.h></div><div>+#include <unistd.h></div><div>+#include <errno.h></div><div>+</div><div>+#define sample_dir "getcwd.sample_dir"</div><div>+#define path_size 2048</div><div>+int</div><div>+main(void)</div><div>+{</div><div>+ char cur_dir[path_size];</div><div>+</div><div>+ if (mkdir(sample_dir, S_IRWXU) == -1)</div><div>+ perror_msg_and_skip("mkdir");</div><div>+</div><div>+ if (chdir(sample_dir) == -1)</div><div>+ perror_msg_and_skip("chdir");</div><div>+</div><div>+ if (getcwd(cur_dir, path_size) == NULL)</div><div>+ perror_msg_and_fail("getcwd");</div><div>+</div><div>+ if (rmdir(cur_dir) == -1)</div><div>+ perror_msg_and_skip("rmdir");</div><div>+</div><div>+ getcwd(cur_dir, path_size);</div><div>+</div><div>+ return 0;</div><div>+}</div><div>diff --git a/tests/getcwd.expected b/tests/getcwd.expected</div><div>new file mode 100644</div><div>index 0000000..301c8ac</div><div>--- /dev/null</div><div>+++ b/tests/getcwd.expected</div><div>@@ -0,0 +1,2 @@</div><div>+getcwd\(\"[^\0]+"\, 2048\) += [0-9]+</div><div>+getcwd\(0x[0-9a-f]+, 2048\) += -1 ENOENT \(No such file or directory\)</div><div>diff --git a/tests/getcwd.test b/tests/getcwd.test</div><div>new file mode 100755</div><div>index 0000000..03fabb1</div><div>--- /dev/null</div><div>+++ b/tests/getcwd.test</div><div>@@ -0,0 +1,9 @@</div><div>+#!/bin/sh</div><div>+</div><div>+. "${srcdir=.}/init.sh"</div><div>+</div><div>+run_prog > /dev/null</div><div>+run_strace -egetcwd $args</div><div>+match_grep</div><div>+</div><div>+exit 0</div><div>-- </div><div>1.9.1</div></div></div></div></div>