[LTP] [PATCH 0/2] safe_stdio: Added SAFE_POPEN() + new error message format

[LTP] [PATCH 0/2] safe_stdio: Added SAFE_POPEN() + new error message format

[Mats Liljegren]

These patches are in preparation of v3 of partrt_nohz_full patch.

Mats Liljegren (2):
  SAFE_POPEN: Added function to safe_stdio.h
  safe_stdio: Error messages in new format

 include/safe_stdio.h |    5 +++++
 lib/safe_stdio.c     |   32 +++++++++++++++++++++++++++++---
 2 files changed, 34 insertions(+), 3 deletions(-)

-- 
1.7.10.4


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 1/2] SAFE_POPEN: Added function to safe_stdio.h

[Mats Liljegren]

Signed-off-by: Mats Liljegren <mats.liljegren@enea.com>
---
 include/safe_stdio.h |    5 +++++
 lib/safe_stdio.c     |   25 +++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/safe_stdio.h b/include/safe_stdio.h
index a4e3da0..b006567 100644
--- a/include/safe_stdio.h
+++ b/include/safe_stdio.h
@@ -36,4 +36,9 @@ int safe_asprintf(const char *file, const int lineno, void (cleanup_fn)(void),
 #define SAFE_ASPRINTF(cleanup_fn, strp, fmt, ...) \
 	safe_asprintf(__FILE__, __LINE__, cleanup_fn, strp, fmt, __VA_ARGS__)
 
+FILE *safe_popen(const char *file, const int lineno, void (cleanup_fn)(void),
+		 const char *command, const char *type);
+#define SAFE_POPEN(cleanup_fn, command, type) \
+	safe_popen(__FILE__, __LINE__, cleanup_fn, command, type)
+
 #endif /* __SAFE_STDIO_H__ */
diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
index d9a2529..b61daa5 100644
--- a/lib/safe_stdio.c
+++ b/lib/safe_stdio.c
@@ -19,6 +19,7 @@
 #define _GNU_SOURCE
 #include <stdarg.h>
 #include <stdio.h>
+#include <errno.h>
 #include "test.h"
 #include "safe_stdio.h"
 
@@ -68,3 +69,27 @@ int safe_asprintf(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	return ret;
 }
+
+FILE *safe_popen(const char *file, const int lineno, void (cleanup_fn)(void),
+		 const char *command, const char *type)
+{
+	FILE *stream;
+	const int saved_errno = errno;
+
+	errno = 0;
+	stream = popen(command, type);
+
+	if (stream == NULL) {
+		if (errno != 0) {
+			tst_brkm(TBROK | TERRNO, cleanup_fn,
+				 "%s:%d: popen(%s,%s) failed",
+				 file, lineno, command, type);
+		} else {
+			tst_brkm(TBROK, cleanup_fn,
+				 "%s:%d: popen(%s,%s) failed: Out of memory",
+				 file, lineno, command, type);
+		}
+	}
+
+	errno = saved_errno;
+}
-- 
1.7.10.4


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 2/2] safe_stdio: Error messages in new format

[Mats Liljegren]

safe_macros.c introduced a new format of error messages using the following
syntax:
<file>:<line> <function>(<parameters>) failed <optional extra message>

This patch will introduce this format to safe_stdio.c as well.

Signed-off-by: Mats Liljegren <mats.liljegren@enea.com>
---
 lib/safe_stdio.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
index b61daa5..47966c5 100644
--- a/lib/safe_stdio.c
+++ b/lib/safe_stdio.c
@@ -30,7 +30,8 @@ FILE *safe_fopen(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (f == NULL) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "fopen(%s) failed at %s:%d", path, file, lineno);
+			 "%s:%d: fopen(%s,%s) failed",
+			 file, lineno, path, mode);
 		return NULL;
 	}
 
@@ -46,7 +47,7 @@ int safe_fclose(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (ret) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "fclose failed at %s:%d", file, lineno);
+			 "%s:%d: fclose(%p) failed", file, lineno, f);
 	}
 
 	return ret;
@@ -64,7 +65,7 @@ int safe_asprintf(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (ret < 0) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "asprintf failed at %s:%d", file, lineno);
+			 "%s:%d: asprintf(%s,...) failed", file, lineno, fmt);
 	}
 
 	return ret;
-- 
1.7.10.4


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/2] SAFE_POPEN: Added function to safe_stdio.h

[chrubis]

Hi!
> +FILE *safe_popen(const char *file, const int lineno, void (cleanup_fn)(void),
> +		 const char *command, const char *type)
> +{
> +	FILE *stream;
> +	const int saved_errno = errno;
> +
> +	errno = 0;
> +	stream = popen(command, type);
> +
> +	if (stream == NULL) {
> +		if (errno != 0) {
> +			tst_brkm(TBROK | TERRNO, cleanup_fn,
> +				 "%s:%d: popen(%s,%s) failed",
> +				 file, lineno, command, type);
> +		} else {
> +			tst_brkm(TBROK, cleanup_fn,
> +				 "%s:%d: popen(%s,%s) failed: Out of memory",
> +				 file, lineno, command, type);
> +		}
> +	}
> +
> +	errno = saved_errno;

I've added missing return stream; here and pushed both patches, thanks.

> +}

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/2] SAFE_POPEN: Added function to safe_stdio.h

[chrubis]

Hi!
> > I've added missing return stream; here and pushed both patches,
> > thanks.
> 
> Whoa, did I really miss that? Wondering how my test could have
> run... Must have used the wrong version of the code when testing or
> something.

I've seen cases where such code was running fine because the register
used to return function value was the same that was allocated for the
temporary variable...

> But thanks for the merge!
> 
> Now you know why I really liked forcing people into using
> -Werror ;-) Could it be a good idea to actually use -Werror by default,
> and having a "production" flag for those who don't want it? Just to
> ensure that lazy people are those being best looked after...

I would be for reversing the logic, i.e. adding developer flag that
enables -Werror. Unfortunatelly LTP has tons of legacy code that
produces megatons of warnings and fixing all of these would be a big
effort.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/2] SAFE_POPEN: Added function to safe_stdio.h

[chrubis]

Hi!
> > > > I've added missing return stream; here and pushed both patches,
> > > > thanks.
> > > 
> > > Whoa, did I really miss that? Wondering how my test could have
> > > run... Must have used the wrong version of the code when testing or
> > > something.
> > 
> > I've seen cases where such code was running fine because the register
> > used to return function value was the same that was allocated for the
> > temporary variable...
> 
> I thought about that alternative but discarded it as too unlikely, but
> apparently, I shouldn't have been so quick about it apparently...
> 
> > > Now you know why I really liked forcing people into using
> > > -Werror ;-) Could it be a good idea to actually use -Werror by
> > > default, and having a "production" flag for those who don't want
> > > it? Just to ensure that lazy people are those being best looked
> > > after...
> > 
> > I would be for reversing the logic, i.e. adding developer flag that
> > enables -Werror. Unfortunatelly LTP has tons of legacy code that
> > produces megatons of warnings and fixing all of these would be a big
> > effort.
> > 
> 
> Hmm, but a compromise could be this:
> 
> Let's say we have a "production flag" which then each module could make
> their mind up whether it should have any effect or not. If it should
> have an effect, then the effect should be to disable -Werror.

I'm still not convinced that enabling -Werror by default is a good idea.
Compiler warnings are quite unstable among different versions and code
that produces no warnings with one version is not guaranteed not to
produce anything with another, and there are false positives too.

Cloning LTP from git, running configure and make must not fail just
because you have different compiler version than developers.

> This way, you can fix warnings on one module at a time, no big bang
> change. When a module is fixed, you change it to use -Werror unless
> this "production flag" is used.

That sounds to me as too much work for not so much gain.

I guess that we can add a configure parameter that adds a variable into
config.mk something as WERROR which gets set to -Werror only if it's
supported by compiler and enabled by user. Then we can do
CFLAGS+=$(WERROR) in a few Makefiles.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list