On Tue, Oct 21, 2014 at 09:20:34PM +0400, Ilya Verbin wrote:
> This patch contains liboffloadmic library.
> 
> It is used by ICC for offloading.  The sources are imported from upstream
> ( https://www.openmprtl.org/sites/default/files/liboffload_oss.tgz )
> Configure and makefiles are new.
> 
> Also liboffloadmic/runtime/emulator directory is new.  This emulator consists
> of 4 shared libraries which replace COI and MYO libraries from MPSS stack.

For the real offloading rather than emulation, are there any further
libraries needed, or are the COI and MYO bits included in the source
everything that is needed?  Does one need some kernel module, will it be
open source, are there any plans to push it to Linus' tree?
I know the HW isn't shipping yet, but it would be nice to get those
questions answered now.

In git, I see that liboffloadmic/Makefile.am has -rwxrwxr-x permissions,
please avoid that, only use executable permissions for shell scripts (so,
configure is fine and desirable).

>From quick look at git, write_message has been supposedly fixed to use
va_list, but I see it is wrong:

    void write_message(FILE * file, int msgCode, va_list args_p) {
        va_list args;
        char buf[1024];

        va_copy(args, args_p);
        buf[0] = '\r';
        vsnprintf(buf + 1, sizeof(buf), MESSAGE_TABLE_NAME[ msgCode ], args);
        strcat(buf, "\n");
        va_end(args);
        fputs(buf, file);
        fflush(file);
    }

As vsnprintf's first argument is buf + 1, it might happily overflow the
buffer, and if not that, then the strcat could too.
So, I'd expect sizeof(buf) - 2 be needed there instead of sizeof(buf).
Also, do we really want the messy DOS/Windows '\r' in the messages on
Unix-ish targets?  Shouldn't that be dependent on what target is the library
configured for?

        Jakub

Reply via email to