Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4E3A85E1.6010008@gmail.com>
Date: Thu, 04 Aug 2011 13:43:29 +0200
From: Luka Marčetić <paxcoder@...il.com>
To: musl@...ts.openwall.com
Subject: Re: cluts weekly reports

On 08/04/2011 12:53 AM, Rich Felker wrote:
>
> OK, I'm imagining something like this:
>
> void test_realpath(const struct test *test, void *res, char *buf, size_t len)
> {
> 	*(char *)res = realpath(test->arg[0].ptr, buf);
> }
>
> struct test tests[] = {
> { .function = test_realpath, .arg[0].ptr = ".", ... },
> { .function = test_realpath, .arg[0].ptr = "./123456789", ... },
> ...};

That's even more syntax (though I did use this/similar approach in 
numeric.c). It may be a bit more readable, but it introduces more 
repetitive work, whereas the generator is designed so that it would be 
reduced. In the above example you also suppose the arg member of struct 
test to have a determined, maximal possible length (overhead). I'm 
guessing .ptr should be a void pointer, in which case you'll need (type 
[]) wherever the type of the argument is not char *, and you'd need 
casting back at where you call the function.
Anyway, I do understand where you're coming from, but this is no news to 
me, and it really does nothing to eliminate the need I feel for a code 
generator.

> With some extra fields to indicate the return type and how the caller
> should validate the result. Actually I would make a function (called
> do_test or something) that would do all that work.

Exactly, but this wouldn't help at all. If anything, it'd introduce more 
code. As I've said before, what main does would just go into that new 
function. This is what happens with numeric.c, but it would be even 
worse, since the functions tested may differ greatly which would make 
do_test even larger.

> [...]

> The result handling could also be better. Actually you might want to
> consider having the test structure include a "validate" function
> pointer that would be used to validate the results.

Another function which would do main's work, and introduce an overhead 
of having to be called only to have more code written (type casting, 
return, etc).

> [...]
> In addition, the generic code could always check errno unless you have
> a flag not to check it,

In numeric.c I do that by setting expected errno to -1

> This is all very general stuff I whipped up in 30 minutes or so. I
> could elaborate on it if this isn't giving you enough ideas.

Nothing that I already haven't come up with.
I did use function pointers once too, but compiler complained, so I went 
back to switch-cases.

> Also, if you think this isn't helpful, please expand on the example
> you sent me. Seeing ONE TEST doesn't give me any idea of the type of
> generality you're trying to achieve.

I thought it was obvious: You write test data into a nice, clean little 
json structure (and include a function prototype). The generator parses 
the prototype, and generates most/all of the code needed for the test to 
go through.
To understand how you could expand the example, you can imagine you add 
another dataset for the same function into the json file (which requires 
a different return result for eg.). This would have an effect of 
generator adding to (struct data []). Likewise, if you add a whole new 
testset and specify a different function prototype altogether, a new 
element to t[] would be added, as well as a new case with correct 
comparisons etc.

> Also..
>
>>      for (f=0; f<sizeof(t)/sizeof(t[0]); ++f) {
>>          memset(&error, 0, sizeof(error));
>>          sigaction(SIGSEGV,&act,&oldact);
>>          sig = 0;
>>          for (d=0; !(sig = setjmp(env))&&  !memcmp(&error,&no_error, sizeof(error))&&   d<t[f].ndata; ++d) {
>>              arg = t[f].data[d].arg; //shorthand args
>>              for (i=0; !memcmp(&error,&no_error, sizeof(error))&&  i<iters(t[f],d); ++i) {
>>                  switch (f) {
>>                          case 0:
>>                              if ((ret.s = realpath(ARGV(char *, 0), ARGV(char *, 1))) != *(char **)t[f].data[d].ret) {
> This is certainly wrong, in general. You can't use equality operators
> to compare strings, but perhaps you're just looking for null pointers
> here?

Don't be so certain. The generator would use strcmp if the expected 
return value is not NULL (and in case of error would print it with 
\"%s\" rather than %d which it does there).

> Also the second arg needs to be a caller-provided buffer, but

"If the/resolved_name/argument is a null pointer, the pointer returned 
by/realpath/() can be passed to/free/()."
Granted, where caller-provided buffers are required, a need would arise 
to modify the generated code manually (and that's not the only occasion).


> part of the test structure, I think.. I'm a bit confused how this code
> is supposed to work.

This code would actually require a chdir() to be added to it to change 
into a directory so deep that when resolved_name is called, PATH_MAX is 
exceeded, and errno should be set to ENAMETOOLONG. It's one of the tests 
for the task nr. 7.

Luka.
P.S. I guess I'll go with signals test for now?

Content of type "text/html" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.