[LTP] [PATCH] syscalls/mmap16: close open files in cleanup path

Cyril Hrubis chrubis@suse.cz
Tue Jul 12 15:38:24 CEST 2016


Hi!
> > after the SAFE_CLOSE() line? doc/test-writing-guidelines.txt
> > says "The SAFE_CLOSE() function also sets the passed file
> > descriptor to -1 after it's successfully closed.", so if
> > that's not correct we should fix the docs.
> 
> It's true for newlib, but not for oldlib. I think we should
> rather fix oldlib SAFE_CLOSE to match the docs, so I dropped
> the line:
> 
> include/old/safe_macros.h:
> #define SAFE_CLOSE(cleanup_fn, fildes)  \
>         safe_close(__FILE__, __LINE__, (cleanup_fn), (fildes))
> 
> I'll send a patch for oldlib SAFE_CLOSE.

I avoided changing the oldlib in order not to introduce any regressions.
There may be code that depends on the fd > 0 even after the close, it's
unlikely but possible.

I'm OK with changing it, I just avoided to do so in the patch that added
the new library API.

> > That doc also gives an example (in section 2.2.1) that
> > says "Since global variables are initialized to zero we can
> > just check that fd > 0 before we attempt to close it.",
> > which is why I used 0 rather than -1. If current preferred
> > test style has changed it would be nice to update the
> > example code.
> 
> Hmm. Cyril, can we guarantee that testcase won't open fd 0?
> Would that apply for both newlib and oldlib?

Well the fd 0 always points to STDIN, so unless the test actively closes
it this will work. There may be testcases that does that, and would
require special treatement, but I'm pretty sure that these are in single
numbers.

-- 
Cyril Hrubis
chrubis@suse.cz


More information about the ltp mailing list