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

[chrubis@suse.cz]

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