again, code snippet without comment

[Phil Carmody]

Say I wrote:

// includes elided
int code_to_talk_to_chip_specific_copro(int funcid, ...)
{
va_list va;
uint32_t addr;
int ret;
va_start(va, funcid);
addr = *(uint32_t*)&va;
switch(funcid)
{
case do_X:
ret = get_copro_to_do_job_X(addr);
break;
// others
}
va_end(va);
}

Would you pat me on the back for cleverly getting the address of
nameless parameters, or would you get out your cane and say
"wroooong, do it again!".

Phil

 

[Seebs]

Would you pat me on the back for cleverly getting the address of
nameless parameters, or would you get out your cane and say
"wroooong, do it again!".

The latter.

-s

 

[Kaz Kylheku]

Say I wrote:

// includes elided
int code_to_talk_to_chip_specific_copro(int funcid, ...)
{
va_list va;
uint32_t addr;
int ret;
va_start(va, funcid);
addr = *(uint32_t*)&va;
switch(funcid)
{
case do_X:
ret = get_copro_to_do_job_X(addr);

Does this function copy the arguments to some request
structure which is fed to the coprocessor, or does
it pass the memory directly to the coprocessor?

If it's the latter, it has to block until the it does the job,
which takes out some of the ``co'' out of ``copro''.

If the function copies the arguments, surely you can
write a va_list variant of it which copies arguments
out of the va_list using the proper va_arg access macros.

int va_get_copro_to_do_job_X(va_list args)
{
copro_request_t *rq = copro_alloc_blank_request();

if (rq == 0)
/* return error code? */;

/* extract whatever properties go with this operation */

rq->type = COPRO_JOB_X;
rq->job_X.addr = va_arg (args, uint32_t);
rq->job_X.size = va_arg (args, uint32_t);

return copro_submit_request(rq);
}

 

[Eric Sosman]

Say I wrote:

// includes elided
int code_to_talk_to_chip_specific_copro(int funcid, ...)
{
va_list va;
uint32_t addr;
int ret;
va_start(va, funcid);
addr = *(uint32_t*)&va;
switch(funcid)
{
case do_X:
ret = get_copro_to_do_job_X(addr);
break;
// others
}
va_end(va);
}

Would you pat me on the back for cleverly getting the address of
nameless parameters, or would you get out your cane and say
"wroooong, do it again!".

I'd vote for "wroooong," on at least three counts. First,
the alignment of `va' might not meet the requirements for an
uint32_t. Second, a va_list might be larger than a uint32_t, so
you'd only be passing a fragment of it. Finally, va_list is an
"opaque" type, whose internals are not specified by the Standard
and need not even be documented by the implementation: You don't
know how to use what you've retrieved, even if you've retrieved
it successfully.

You certainly haven't got "the address of nameless
parameters," which may well reside in unaddressable places.

 

[Alan Curry]

Say I wrote:

// includes elided
int code_to_talk_to_chip_specific_copro(int funcid, ...)
{
va_list va;
uint32_t addr;
int ret;
va_start(va, funcid);
addr = *(uint32_t*)&va;
switch(funcid)
{
case do_X:
ret = get_copro_to_do_job_X(addr);
break;
// others
}
va_end(va);
}

Would you pat me on the back for cleverly getting the address of
nameless parameters, or would you get out your cane and say
"wroooong, do it again!".

Why bother with va_* at all, if you're going to use some arch-specific
trickery to get the address you need? Something like this:

addr = (uint32_t)(&funcid+1);

would do the same non-portable job, but in a way that can be understood
directly by anyone who knows how arguments are passed in the ABI you're
targeting. Your version requires the reader to know not only that but also
the layout of va_list.

 

[Phil Carmody]

The latter.

Many thanks!

Not for the caning, but for being non-anonymous justification for
the caning I'd already handed out. The excuse "but I copied that
bit off ..." meant that I'd inadvertantly caned 2 people, but the
originator took it like a man. (There was also mention of "it's
obviously &funcid+1", much to my simultanious delight and chagrin.)

Thanks all, of course.

Phil

 

[Michael Foukarakis]

Say I wrote:

// includes elided
int code_to_talk_to_chip_specific_copro(int funcid, ...)
{
    va_list va;
    uint32_t addr;
    int ret;
    va_start(va, funcid);
    addr = *(uint32_t*)&va;
    switch(funcid)
    {
        case do_X:
            ret = get_copro_to_do_job_X(addr);
            break;
       // others
    }
    va_end(va);

}

Would you pat me on the back for cleverly getting the address of
nameless parameters, or would you get out your cane and say
"wroooong, do it again!".

I'd say "That may work, but since I don't care for finding out how a
va_list is organized, why don't you write it in a way that a) will
work every time and b) someone else can understand it ?".

 

[Phil Carmody]

Does this function copy the arguments to some request
structure which is fed to the coprocessor, or does
it pass the memory directly to the coprocessor?

Passed directly.

If it's the latter, it has to block until the it does the job,
which takes out some of the ``co'' out of ``copro''.

Not really, your abstract machine may be an abacus, but out here
in the real world we have multiple threads. The client always has
to wait for any coprocessor which produces a result, there's nothing
new about that idea. It doesn't stop them being coprocessors.

If the function copies the arguments, surely you can
write a va_list variant...

Given that the layout of the data had to satisfy the copro's
interface spec, the opaqueness of va_list and the arbitrariness
of the main core's ABI were two things I think reliance on
which should be avoided.

Damn I'm tired (bloody DIY maniac upstairs depriving me of
sleep) I don't even think that previous paragraph parses.

Phil