Andy Canfield
2001-04-29 06:55:46 UTC
Last year I had trouble getting isc_expand_dpb() to work. It turns out that it CAN'T work as described in the manual. The problems are in the interface, not in the code.
What's wrong with isc_expand_dpb() -
[1] There is a difference between the size of a buffer and the size of the data in the buffer. The API says that isc_expand_dpb() allocates a new buffer if the new material will not fit into the old buffer. However, the length parameter is the size of the data in the buffer; isc_expand_dbp() has no way of knowing how big the old buffer is.
[2] The API says that isc_expand_dpb() allocates a new buffer if it needs it. However, there is nothing said about what happens to the old buffer. There is no deallocation of the old buffer.
Because these flaws are in the functional specs of isc_expand_dpb(), and not in the code itself, I propose to repair them by providing two new functions:
isc_fill_dpb() would exactly four parameters:
A pointer to a DBP. Before the first call to isc_fill_dbp(), the caller must set this pointer to NULL. isc_fill_dbp() will do all allocating of the dbp buffer.
A pointer to a short int giving the length of the data in the dbp_buffer. This is purely an output of isc_fill_dpb(), and is provided solely for compatablity with isc_attach_database(), which needs this number as a parameter.
A parameter code for the string that is to be inserted into the dbp.
A pointer to the string that is to be inserted into the dbp.
Note that isc_fill_dbp() would not take variable arguments, like isc_expand_dpb(). Variable arguments bypass compile-time type checking, and thus are error-prone.
isc_free_dpb() takes exactly one parameter, the pointer to the DPB as created and filled by isc_fill_dbp(). This must be called AFTER the call to isc_attach_database, to release the memory in the buffer.
Code calling these function would look like this:
char * username = ...;
char * password = ...;
char * dpb = NULL;
short dbp_length;
isc_fill_dpb( & dpb, & dpb_length, isc_dpb_user_name, username );
isc_fill_dpb( & dpb, & dpb_length, isc_dpb_password, password );
isc_attach_database( & status_vector, strlen( dbname ), dbname,
& dbhandle, dpb_length, dbp );
isc_free_dpb( dbp );
To implement this I propose a new dpb code which the caller never sees and which marks the allocated length of the dpb and the actual data length of the dbp (two numbers). Because shorts look a lot smaller now than they did a few years ago, I'd like to make these four byte numbers. Therefore the length information block would be ten bytes:
one byte for isc_dpb_length
one byte with the value 9
a four byte unsigned integer, the allocated length, standard (VAX?) format
a four byte unsigned integer, the data length, in standard (VAX?) format
I believe that this implementation meets the following criteria:
* It constructs a dbp which, with the exception of the new isc_dpb_length field, is completely compatable with the existing dpb format. I am led to understand that isc_attach_database() ignores any dpb code that it doesn't understand; therefore it will ignore the isc_dpb_length code and work as before.
* It can be fully thread-friendly. Indeed, one application can construct two dbp's at the same time, interleaving the calls to isc_fill_dbp() at will, with no clash.
* There is no memory leak (unless the caller forgets the call to isc_free_dpb()). No matter how many times the caller calls isc_free_dbp(), no matter how many times isc_free_dbp() must re-allocate the dbp buffer; because isc_free_dpb() knows how the buffer was allocated, it will free it whenever it must replace it.
* with fixed parameters to isc_free_dpb(), not variable parameters, we have type checking on the code.
* When we wish to, we can upgrade isc_attach_database() to store the dpb_length in an unsigned long integer, and recognize the isc_dpb_length code, updating the (small) number it received from the caller with the (large) number it finds in the dbp itself. When we do this we will no longer be limited to 32767 bytes in the dbp.
I am a little concerned that the length of a string in the dbp buffer is limited to 255 characters by the use of one byte as a length byte; file names under Windows 98 can be as long as 32K bytes. If the user calls isc_fill_dpb() to put the strings into the dbp, isc_fill_dpb() can set the format code to "2" and make the upgrade invisible to the caller.
Of course we need to keep isc_expand_dpb() in the API to support old code; however we should discourage it's use for new code and shift over to the new functions.
If the user wants to "roll his own" dpb the old format should be specified; the new format should be hidden inside isc_fill_dpb() and isc_attach_database().
I would not hold up Firebird Relase 1 to do this; it can go into a later release.
I would expect to write these myself, but I'm not sure I can build the package since I don't own M$VC and I can only test it on Win98.
I haven't looked at the details, but there are TPB's (Transaction Parameter Buffers) and SPB's (Service Parameter Buffers). Assuming they are similar, I would design isc_fill_pb() and isc_free_pb() to handle any of the three.
I welcome any comments and suggestions and plans of others.
What's wrong with isc_expand_dpb() -
[1] There is a difference between the size of a buffer and the size of the data in the buffer. The API says that isc_expand_dpb() allocates a new buffer if the new material will not fit into the old buffer. However, the length parameter is the size of the data in the buffer; isc_expand_dbp() has no way of knowing how big the old buffer is.
[2] The API says that isc_expand_dpb() allocates a new buffer if it needs it. However, there is nothing said about what happens to the old buffer. There is no deallocation of the old buffer.
Because these flaws are in the functional specs of isc_expand_dpb(), and not in the code itself, I propose to repair them by providing two new functions:
isc_fill_dpb() would exactly four parameters:
A pointer to a DBP. Before the first call to isc_fill_dbp(), the caller must set this pointer to NULL. isc_fill_dbp() will do all allocating of the dbp buffer.
A pointer to a short int giving the length of the data in the dbp_buffer. This is purely an output of isc_fill_dpb(), and is provided solely for compatablity with isc_attach_database(), which needs this number as a parameter.
A parameter code for the string that is to be inserted into the dbp.
A pointer to the string that is to be inserted into the dbp.
Note that isc_fill_dbp() would not take variable arguments, like isc_expand_dpb(). Variable arguments bypass compile-time type checking, and thus are error-prone.
isc_free_dpb() takes exactly one parameter, the pointer to the DPB as created and filled by isc_fill_dbp(). This must be called AFTER the call to isc_attach_database, to release the memory in the buffer.
Code calling these function would look like this:
char * username = ...;
char * password = ...;
char * dpb = NULL;
short dbp_length;
isc_fill_dpb( & dpb, & dpb_length, isc_dpb_user_name, username );
isc_fill_dpb( & dpb, & dpb_length, isc_dpb_password, password );
isc_attach_database( & status_vector, strlen( dbname ), dbname,
& dbhandle, dpb_length, dbp );
isc_free_dpb( dbp );
To implement this I propose a new dpb code which the caller never sees and which marks the allocated length of the dpb and the actual data length of the dbp (two numbers). Because shorts look a lot smaller now than they did a few years ago, I'd like to make these four byte numbers. Therefore the length information block would be ten bytes:
one byte for isc_dpb_length
one byte with the value 9
a four byte unsigned integer, the allocated length, standard (VAX?) format
a four byte unsigned integer, the data length, in standard (VAX?) format
I believe that this implementation meets the following criteria:
* It constructs a dbp which, with the exception of the new isc_dpb_length field, is completely compatable with the existing dpb format. I am led to understand that isc_attach_database() ignores any dpb code that it doesn't understand; therefore it will ignore the isc_dpb_length code and work as before.
* It can be fully thread-friendly. Indeed, one application can construct two dbp's at the same time, interleaving the calls to isc_fill_dbp() at will, with no clash.
* There is no memory leak (unless the caller forgets the call to isc_free_dpb()). No matter how many times the caller calls isc_free_dbp(), no matter how many times isc_free_dbp() must re-allocate the dbp buffer; because isc_free_dpb() knows how the buffer was allocated, it will free it whenever it must replace it.
* with fixed parameters to isc_free_dpb(), not variable parameters, we have type checking on the code.
* When we wish to, we can upgrade isc_attach_database() to store the dpb_length in an unsigned long integer, and recognize the isc_dpb_length code, updating the (small) number it received from the caller with the (large) number it finds in the dbp itself. When we do this we will no longer be limited to 32767 bytes in the dbp.
I am a little concerned that the length of a string in the dbp buffer is limited to 255 characters by the use of one byte as a length byte; file names under Windows 98 can be as long as 32K bytes. If the user calls isc_fill_dpb() to put the strings into the dbp, isc_fill_dpb() can set the format code to "2" and make the upgrade invisible to the caller.
Of course we need to keep isc_expand_dpb() in the API to support old code; however we should discourage it's use for new code and shift over to the new functions.
If the user wants to "roll his own" dpb the old format should be specified; the new format should be hidden inside isc_fill_dpb() and isc_attach_database().
I would not hold up Firebird Relase 1 to do this; it can go into a later release.
I would expect to write these myself, but I'm not sure I can build the package since I don't own M$VC and I can only test it on Win98.
I haven't looked at the details, but there are TPB's (Transaction Parameter Buffers) and SPB's (Service Parameter Buffers). Assuming they are similar, I would design isc_fill_pb() and isc_free_pb() to handle any of the three.
I welcome any comments and suggestions and plans of others.