[Cbe-oss-dev] [PATCH] libspe, libspe2: remove CHECK_nnn_OPCODE to fix waitpid

[Kazunori Asayama]

Patrick Mansfield <patmans at us.ibm.com> wrote:
> The waitpid handler is checking for the wrong opcode, it should be
> CHECK_POSIX1_OPCODE(WAITPID) not CHECK_POSIX1_OPCODE(WAIT).
> 
> That could be simply fixed, but on failure returning 1 from the handler
> does not do anything - neither the return code (rc) nor errno are set -
> and the SPE/newlib code ends up with whatever values were passed to the
> assist call (garbage data for errno, and whatever the first argument was
> for the rc). There is no common rc value that can be used as the rc, like
> -1 (for the C99 or POSIX functions).
> 
> So just remove the opcode checks both CHECK_C99_OPCODE and
> CHECK_POSIX1_OPCODE from libspe and libspe2.

The checks are helpful to detect bugs when modify function pointer
tables (default_c99_funcs[] and default_posix1_funcs[]), because the
tables can be easily broken.

So I'd like to leave the checks or to have more 'robust' initializers
of the tables. I propose two options:

  a. add a new argument which means 'error rc', to CHEKC_*_OPCODE macros:

#define CHECK_C99_OPCODE(_op, __error_rc)                               \
	if (SPE_C99_OP(opdata) != SPE_C99_ ## _op) { 			\
		DEBUG_PRINTF("OPCODE (0x%x) mismatch.\n", SPE_C99_OP(opdata)); 	\
		PUT_LS_RC(__error_rc, 0, 0, ENOSYS);			\
		return 1;                                       	\
	}

  b. remove the opcode checks and modify the initializers:

int (*default_c99_funcs[SPE_C99_NR_OPCODES]) (char *, unsigned long) = {
	[SPE_C99_CLEARERR]	= default_c99_handler_clearerr,
	[SPE_C99_FCLOSE]	= default_c99_handler_fclose,

What do you think ?

--
(ASAYAMA Kazunori
  (asayama at sm.sony.co.jp))
t