Re: [SSSD] [PATCH] 1332-Abort unit test when open() fails

[Ariel Barria]

Thanks to comments

From: sgallagh(a)redhat.com
To: sssd-devel(a)lists.fedorahosted.org
Date: Mon, 14 May 2012 07:22:16 -0400
Subject: Re: [SSSD] [PATCH] 1332-Abort unit test when open() fails

On Mon, 2012-05-14 at 10:22 +0200, Jakub Hrozek wrote:
>  On Sun, May 13, 2012 at 04:18:39PM -0500, Ariel Barria wrote:
>  >    https://fedorahosted.org/sssd/ticket/1332
>  
>  > +    if ( fd < 0 ){
>  
>  Thanks for the patch Ariel, but would you also mind amending the
>  whitespace? We don't put whitespace between brackets and the expression
>  inside, but we do put a space after a normal bracket and a curly one. So
>  the "if" should read:
>  
>  + if (fd < 0) {
>  
>  > +       fail("Cannot open /dev/zero");
>  > +    }else{
>  
>  Similar here, space between curly bracket and else on both sides.
>  
>  The same applies to the second change.
>  
>  I was also thinking we may want to introduce a new macro that would mark
>  the test as failed and quit the test at the same time, something like
>  quit_if and equivalent quit_unless. These would be just simple wrappers
>  around existing fail_if/unless. Along the lines of (untested):
>  
>  #define quit_if(expr, ...) do {    \
>      fail_if(expr, ## __VA_ARGS__); \
>      if (expr) {                    \
>          return;                    \
>      }                              \
>  } while(0)
>  
>  But perhaps that's outside the scope of this ticket.. 
 
This is the definition of _fail_unless() (which the fail_unless and
fail_if macros wrap):
 
 
void _fail_unless (int result, const char *file,
                   int line, const char *expr, ...)
{
  const char *msg;
    
  send_loc_info (file, line);
  if (!result) {
    va_list ap;
    char buf[BUFSIZ];
    
    va_start(ap,expr);
    msg = (const char*)va_arg(ap, char *);
    if (msg == NULL)
      msg = expr;
    vsnprintf(buf, BUFSIZ, msg, ap);
    va_end(ap);
    send_failure_info (buf);
    if (cur_fork_status() == CK_FORK) {
#ifdef _POSIX_VERSION
      exit(1);
#endif /* _POSIX_VERSION */
    }
  }
}
 
 
So basically, as long as we're running with CK_FORK=yes (the default
mode), failing a test is already guaranteed to exit.
 
We might want to propose a patch upstream that exit(1) should not be
contingent upon CK_FORK, since I would be willing to bet that in most
programs, continuing on would result only in chaining failures.
 
I do understand, however, that their goal here is to allow running ALL
the tests so they can display only the failures at the end.
 
Also keep in mind that CK_FORK=no is intended only as a debugging mode
(so it's easier to get into gdb/valgrind). All of the check tests are
written with CK_FORK=yes in mind, because exiting the child process
guarantees cleanup, where if you are using CK_FORK=no, your state may
not be stable between individual tests.

_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel