[OPEN-ILS-DEV] C nits: utils.c (Part 2)

[David J. Fiander]

> On 3/22/07, *Scott McKellar* <mck9 at swbell.net <mailto:mck9 at swbell.net>> 
> wrote:
>     A second pedantic point: In your error message you format
>     BUFFER_MAX_SIZE as an int, with the "%d" format.  I'm not sure what
>     type the compiler applies to an integer literal, such as
>     BUFFER_MAX_SIZE expands to.  Probably it's an int unless it's too
>     big to be an int.

Well, BUFFER_MAX_SIZE is not necessarily an integer literal; it might (and 
probably does) contain a cast, probably to size_t.

> 
>     In any case there are systems where the value of BUFFER_MAX_SIZE won't
>     fit into an int.  Probably none of them would ever run Open-ILS.

Imagine a transitional 64-bit capable system:

	short: 16 bits
	int:   32 bits
         long:  64 bits

If pointers are 64 bits long, then size_t might have to be a long as well 
(although I worked on a system with 32 bit ints and ptrdiff_t; and 64 bit 
pointers, because the pointers had lots of type information encoded in them).

> 
>     In any case, the pedantically correct thing to do is to use the "%ld"
>     format, and cast BUFFER_MAX_SIZE to a long.  Alternatively you could
>     append an "L" to the definition of this macro.  You could also use
>     the "%lu" format and turn BUFFER_MAX_SIZE to an unsigned long.

One of the problems with the C format codes is the assumption that the type 
system is simple, but yes, %lu is probably the best format in general.

>     You would likely want to wrap realloc() in a safe_realloc function, or
>     an OSRF_REALLOC macro.

The only reason that realloc() might fail is because there's not enough 
memory for the new buffer size.  I suspect that if we get into that 
situation, the simplest thing to do is to die:

	if ((p = realloc(p, newsize)) == NULL) {
		die("ack!");
	}

> 
>     Finally: when you append the string to the buffer, use strcpy()
>     instead of strcat():
> 
>         strcpy( gb->buf + gb->n_used, data );
> 
>     Rationale: strcat() has to spend CPU cycles finding the end of the
>     original string.  But you already know where the end of the string
>     is.  You might as well put that information to good use.

Does the count gb->n_used include the '\0' at the end of the string, or 
not?  A zero-length string stored in a buffer is using one byte of the 
buffer, but to concatenate a string onto the zero-length string we need to 
adjust the source pointer by 0, not by 1 (induction is wonderful).  The 
nice thing about strcat() is that it eliminates the need to worry about 
this sort of fussy detail, even if it does cost a few processor cycles.

- David

-- 
David J. Fiander
Digital Services Librarian