[PATCH 1/3] t_mtab: Add newlines to error messages

[PATCH 1/3] t_mtab: Add newlines to error messages

[Tuomas Tynkkynen]

I can't recall anymore what exactly I did to have tests using t_mtab
to fail, but nevertheless this commit adds the proper newlines.

Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>
---
 src/t_mtab.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/t_mtab.c b/src/t_mtab.c
index 9bca9543..5e9d5216 100644
--- a/src/t_mtab.c
+++ b/src/t_mtab.c
@@ -34,7 +34,7 @@ static int signals_have_been_setup = 0;
 /* Ensure that the lock is released if we are interrupted.  */
 static void
 handler (int sig) {
-    fprintf(stderr, "%s", sys_siglist[sig]);
+    fprintf(stderr, "%s\n", sys_siglist[sig]);
     exit(1);
 }
 
@@ -99,7 +99,7 @@ lock_mtab (void) {
 			   Too many files open in the system?
 			   Filesystem full? */
 			fprintf(stderr, "can't create lock file %s: %s "
-			     "(use -n flag to override)",
+			     "(use -n flag to override)\n",
 			     linktargetfile, strerror (errsv));
 			exit(1);
 		}
@@ -112,7 +112,7 @@ lock_mtab (void) {
 
 		if (j < 0 && errsv != EEXIST) {
 			fprintf(stderr, "can't link lock file %s: %s "
-			     "(use -n flag to override)",
+			     "(use -n flag to override)\n",
 			     mounted_lock, strerror (errsv));
 			exit(1);
 		}
@@ -157,7 +157,7 @@ lock_mtab (void) {
 			if (fcntl (fd, F_SETLKW, &flock) == -1 &&
 			    errno != EBUSY && errno != EAGAIN) {
 				int errsv = errno;
-				fprintf(stderr, "can't lock lock file %s: %s",
+				fprintf(stderr, "can't lock lock file %s: %s\n",
 				     mounted_lock, (errno == EINTR) ?
 				     _("timed out") : strerror (errsv));
 				exit(1);
-- 
2.13.0

[PATCH 2/3] fsx: Fix -Wformat-security warnings

[Tuomas Tynkkynen]

Some distros (NixOS) have their build environment enable
-Werror=format-security by default for security/hardening reasons.
Currently fsx fails to build due to this:

fsx.c: In function 'prt':
fsx.c:215:18: error: format not a string literal and no format arguments [-Werror=format-security]
  fprintf(stdout, buffer);
                  ^
fsx.c:217:20: error: format not a string literal and no format arguments [-Werror=format-security]
   fprintf(fsxlogf, buffer);
                    ^
Indeed the compiler is correct here, if the message-to-be-printed were
to contain a '%', unpredictable things would happen. Fix this.

Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>
---
 ltp/fsx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 3713bbe3..fba2b4d8 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -212,9 +212,9 @@ prt(const char *fmt, ...)
 	va_start(args, fmt);
 	vsnprintf(buffer, BUF_SIZE, fmt, args);
 	va_end(args);
-	fprintf(stdout, buffer);
+	fprintf(stdout, "%s", buffer);
 	if (fsxlogf)
-		fprintf(fsxlogf, buffer);
+		fprintf(fsxlogf, "%s", buffer);
 }
 
 void
-- 
2.13.0

[PATCH 3/3] stat_test, fssum: Include <sys/sysmacros.h> to avoid glibc warning

[Tuomas Tynkkynen]

Compiling against a recent glibc gives a number of warnings of this
sort:

stat_test.c: In function 'cmp_statx':
stat_test.c:251:13: warning: In the GNU C Library, "major" is defined
 by <sys/sysmacros.h>. For historical compatibility, it is
 currently defined by <sys/types.h> as well, but we plan to
 remove this soon. To use "major", include <sys/sysmacros.h>
 directly. If you did not intend to use a system-defined macro
 "major", you should undefine it after including <sys/types.h>.
  devcmp(dev);

Do as the message says to avoid it. xfstests already includes
<sys/sysmacros.h> in two other places so it seems safe to assume this
header generally exists.

Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>
---
 src/fssum.c     | 1 +
 src/stat_test.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/fssum.c b/src/fssum.c
index 04052cdc..fa763d19 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -29,6 +29,7 @@
 #include <fcntl.h>
 #include <dirent.h>
 #include <errno.h>
+#include <sys/sysmacros.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/sysmacros.h>
diff --git a/src/stat_test.c b/src/stat_test.c
index 198c1332..ba1650f2 100644
--- a/src/stat_test.c
+++ b/src/stat_test.c
@@ -17,6 +17,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <sys/sysmacros.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/sysmacros.h>
-- 
2.13.0

Re: [PATCH 2/3] fsx: Fix -Wformat-security warnings

[Amir Goldstein]

On Tue, Aug 29, 2017 at 4:37 PM, Tuomas Tynkkynen
<tuomas.tynkkynen@iki.fi> wrote:
> Some distros (NixOS) have their build environment enable
> -Werror=format-security by default for security/hardening reasons.
> Currently fsx fails to build due to this:
>
> fsx.c: In function 'prt':
> fsx.c:215:18: error: format not a string literal and no format arguments [-Werror=format-security]
>   fprintf(stdout, buffer);
>                   ^
> fsx.c:217:20: error: format not a string literal and no format arguments [-Werror=format-security]
>    fprintf(fsxlogf, buffer);
>                     ^
> Indeed the compiler is correct here, if the message-to-be-printed were
> to contain a '%', unpredictable things would happen. Fix this.
>
> Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>
> ---
>  ltp/fsx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 3713bbe3..fba2b4d8 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -212,9 +212,9 @@ prt(const char *fmt, ...)
>         va_start(args, fmt);
>         vsnprintf(buffer, BUF_SIZE, fmt, args);
>         va_end(args);
> -       fprintf(stdout, buffer);
> +       fprintf(stdout, "%s", buffer);
>         if (fsxlogf)
> -               fprintf(fsxlogf, buffer);
> +               fprintf(fsxlogf, "%s", buffer);
>  }

Might as well be fputs, eitherwise looks good to me

Re: [PATCH 3/3] stat_test, fssum: Include <sys/sysmacros.h> to avoid glibc warning

[Eryu Guan]

On Tue, Aug 29, 2017 at 04:37:15PM +0300, Tuomas Tynkkynen wrote:
> Compiling against a recent glibc gives a number of warnings of this
> sort:
> 
> stat_test.c: In function 'cmp_statx':
> stat_test.c:251:13: warning: In the GNU C Library, "major" is defined
>  by <sys/sysmacros.h>. For historical compatibility, it is
>  currently defined by <sys/types.h> as well, but we plan to
>  remove this soon. To use "major", include <sys/sysmacros.h>
>  directly. If you did not intend to use a system-defined macro
>  "major", you should undefine it after including <sys/types.h>.
>   devcmp(dev);
> 
> Do as the message says to avoid it. xfstests already includes
> <sys/sysmacros.h> in two other places so it seems safe to assume this
> header generally exists.
> 
> Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>
> ---
>  src/fssum.c     | 1 +
>  src/stat_test.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/src/fssum.c b/src/fssum.c
> index 04052cdc..fa763d19 100644
> --- a/src/fssum.c
> +++ b/src/fssum.c
> @@ -29,6 +29,7 @@
>  #include <fcntl.h>
>  #include <dirent.h>
>  #include <errno.h>
> +#include <sys/sysmacros.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <sys/sysmacros.h>

Sorry, I just found this in my release review/testing, "sys/sysmacros.h>
has already been included here, and this was from commit 85e3bf85c67c
("src: include <sys/sysmacros.h> for major() and minor()").

I tested with Fedora rawhide (glibc-2.26.90-9, gcc-7.1.1-7) and I didn't
see this warning when compiling current fstests code. Could you please
help double check if this works for you too with latest fstests? I'll
drop this patch for now.

Thanks,
Eryu

> diff --git a/src/stat_test.c b/src/stat_test.c
> index 198c1332..ba1650f2 100644
> --- a/src/stat_test.c
> +++ b/src/stat_test.c
> @@ -17,6 +17,7 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <sys/sysmacros.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <sys/sysmacros.h>
> -- 
> 2.13.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html