Bug #31916 Inconsistecy between UDF_INIT and UDF_ARGS w/re/to notation of maybe_null
Submitted: 29 Oct 2007 14:47 Modified: 29 Oct 2007 16:03
Reporter: Roland Bouman Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: User-defined functions ( UDF ) Severity:S3 (Non-critical)
Version:5.1 OS:Any
Assigned to: CPU Architecture:Any
Triage: Triaged: D4 (Minor)

[29 Oct 2007 14:47] Roland Bouman
Description:
mysql_com.h contains the definition of the UDF_INIT and UDF_ARGS structures. 
Both contain a member maybe_null, with similar semantics. Both are used to convey that a variable may have a NULL value, and both use a 0 to indicate NOT NULL and 1 to indicate a possible NULL. The only difference is that:

- maybe_null in UDF_ARGS is an array with an entry for each UDF argument
- maybe_null in UDF_ARGS is defined as char *, whereas maybe_null in UDF_INIT  is defined as my_bool:

typedef struct st_udf_args {
...
  char *maybe_null;			/* Set to 1 for all maybe_null args */
...
} UDF_ARGS;

typedef struct st_udf_init
{
  my_bool maybe_null;			/* 1 if function can return NULL */
...
} UDF_INIT;

The problem with UDF_ARGS is that it does not use a notation that expresses its intent. The 

    char *maybe_null 

should be written as a 

    my_bool *maybe_null

to align it with the notation used in UDF_INIT.

In fact, all of UDF_ARGS could be rewritten like this:

typedef struct st_udf_args
{
  unsigned int arg_count;		/* Number of arguments */
  enum Item_result arg_type[];		/* Pointer to item_results */
  char *args[];				/* Pointer to argument value*/
  unsigned long lengths[];		/* Length of string arguments */
  my_bool maybe_null[];			/* Set to 1 for all maybe_null args */
  char *attributes[];                   /* Pointer to attribute name */
  unsigned long attribute_lengths[];    /* Length of attribute arguments */
  void *extension;
} UDF_ARGS;

The array notation has the benefit of explicitly expressing that the members are used as arrays instead of 'just' pointers. This is important, as this is an important part of the interface to an extension point of the server. Cleaning up the notation to indicate the intent of the structure makes it easier and less error prone to work with it.

How to repeat:
see mysql_com.h

Suggested fix:
Rewrite UDF_ARGS as

typedef struct st_udf_args
{
  unsigned int arg_count;		/* Number of arguments */
  enum Item_result arg_type[];		/* Pointer to item_results */
  char *args[];				/* Pointer to argument value*/
  unsigned long lengths[];		/* Length of string arguments */
  my_bool maybe_null[];			/* Set to 1 for all maybe_null args */
  char *attributes[];                   /* Pointer to attribute name */
  unsigned long attribute_lengths[];    /* Length of attribute arguments */
  void *extension;
} UDF_ARGS;
[29 Oct 2007 14:53] Roland Bouman
Additionally, I'd like to point out that the ptr member in UDF_INIT should be written as:

void *ptr;

instead of 

char *ptr;