First, I want to thank everyone for your great replies! I really learned a lot from reading what was written here.

Okay, so I've tried to absorb what I read, and now my XS code looks something like this:

SV *
my_xs_func(p_struct, len)
        void *p_struct;
        IV len
    PREINIT:
        int bytes_read;
        char *tmp_buffer;
    PPCODE:
        tmp_buffer = (char *)malloc(len + 1); // + 1 for NULL
        if (tmp_buffer == NULL)
            XSRETURN_UNDEF;

        bytes_read = my_func(p_struct, tmp_buffer, len);
 
        // need to NULL-terminate the buffer
        tmp_buffer[bytes_read] = '\0';

        XPUSHs(sv_2mortal(newSVpv(tmp_buffer, bytes_read)));

        free(tmp_buffer);
[download]

I have incorporated the advice of Elian here, tye here, and pg here.

I am manipulating the return stack myself, only returning the buffer (the caller can call length), and I'm checking the malloc() return value, and returning undef if it fails. my_func() returns the number of bytes written to tmp_buffer, which is guaranteed to be less than len, so the line that NULL-terminates the buffer should be safe, unless my_func() seriously misbehaves. And I'm using that value for the second argument to newSVpv() instead of zero...

Any new critiques or comments? Does it look better?

Thanks,

3dan