janmojzis / tinyssh

TinySSH is small server (less than 100000 words of code)
Creative Commons Zero v1.0 Universal
1.44k stars 79 forks source link

opentest failed (musl libc) #14

Closed ghost closed 8 years ago

ghost commented 8 years ago
=== Thu May 26 19:42:23 GMT 2016 ===   numtostrtest ok
=== Thu May 26 19:42:23 GMT 2016 ===   opentest failed ... see the log /tmp/tinyssh-master/build/log

build.log:

=== Thu May 26 19:42:23 GMT 2016 ===   newenvtest ok
numtostrtest.c:299:8: warning: integer constant is so large that it is unsigned [enabled by default]
     { -9223372036854775808LL, "-9223372036854775808" },
        ^
numtostrtest.c:299:5: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]
     { -9223372036854775808LL, "-9223372036854775808" },
     ^
=== Thu May 26 19:42:23 GMT 2016 ===   numtostrtest ok
opentest.c:62: process exited with status = 0
ghost commented 8 years ago

it fails on test2/open_write:

curvecp/open_write.c

#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include "open.h"

int open_write(const char *fn)
{
#ifdef O_CLOEXEC
  return open(fn,O_CREAT | O_WRONLY | O_NONBLOCK | O_CLOEXEC,0644);
  ^^^^^^^^^^^
#else
  int fd = open(fn,O_CREAT | O_WRONLY | O_NONBLOCK,0644);
  if (fd == -1) return -1;
  fcntl(fd,F_SETFD,1);
  return fd;
#endif
}

opentest.c

/* test if close-on-exec works for open_write() */
static void test2(void) {

    int fd;

    close(1);
    fd = open_write("opentest.data");
    if (fd != 1) fail("unable to open opentest.data for writing");
       ^^^^^^^
    cat();
}
...
int main(void) {
    alarm(10);
    run_mustfail(test1);
    run_mustfail(test2);
           ^^^^^^^^^^^^^
    run_mustfail(test3);
    run_mustpass(testdummy);
    _exit(0);
}

fix:

if (fd != -1) ? :)
          ^^
janmojzis commented 8 years ago

Hello, thanks for report. Can You describe how did You catch the problem (musl libc version, ...) ?

It looks like problem in libc.

ghost commented 8 years ago

it's neither musl libc nor tinyssh. it's cat utility from embutils

# ls  
cat.c  cat_embutils.c
# cat cat_embutils.c  
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <signal.h>

static int open_write(const char *fn)
{
    return open(fn, O_CREAT | O_WRONLY | O_NONBLOCK | O_CLOEXEC, 0644);
}

static void cat_embutils(void)
{
    char *catcmd[2] = { (char *)"cat_embutils", 0 };
    execvp(*catcmd, catcmd);
}

static void test_embutils(void)
{
    int fd;
    close(1);
    fd = open_write("opentest_embutils.data");

    if (fd != 1)
        printf("unable to open opentest.data for writing");
    cat_embutils();
}

int main()
{
    test_embutils();
    return 0 ;
}
# gcc cat.c -o cat_bb 
# gcc cat_embutils.c -o cat_embutils 
# ls -lh /bin/cat_embutils 
-r-xr-xr-x    1 root     root         9.0k May 28 12:57 /bin/cat_embutils
# ./cat_bb

cat: write error: Bad file descriptor
# echo $?
1
# ./cat_embutils   

cat: -: short write
# echo $?        
0

I dislike this type of errors :)

janmojzis commented 8 years ago

really, 'cat' from embutils doesn't correctly handle 'write' errors.

cat.c (line 85) from embutils-0.19: if (write(1,buf2,len2)<len2) { warn(filename,"short write"); break; }

.... time to rework the opentest without 'cat'

ghost commented 8 years ago

embutil's cat need only one line of code to fix it (res = 1;):

    if (write(1,buf2,len2)<len2) {
      warn(filename,"short write");
      res = 1; /* return fase */
      break;
    }
  }
  if (fd) close(fd);
}