From: Andy C. <an...@ad...> - 2001-04-29 07:51:53
|
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() -=20 [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.=20 [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 =3D ...; char * password =3D ...; char * dpb =3D 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,=20 & 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.=20 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.=20 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().=20 I would not hold up Firebird Relase 1 to do this; it can go into a later = release.=20 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. |
From: Mike N. <ta...@al...> - 2001-04-29 09:00:17
|
Andy Canfield wrote: > What's wrong with isc_expand_dpb() - [snip] > > 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. Instinctively I'd rather see matching create/destroy or alloc/free calls. Other from this change I think your proposal makes sense. Well, perhaps we shouldn't speak about VAX byte ordering anymore. :-) Big endian, little endian, native byte order or network byte order I believe are the "proper" terms. The legacy of the VAX (and I believe sometimes VMS) macro(s) are to be considered deprecated, and should be replaced. The new macros to use would probably be BIG_ENDIAN LITTLE_ENDIAN possibly with "FB_" prepended. > 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 correct, this is indeed a problem waiting to crash and should be logged. /Mike |
From: Andy C. <an...@ad...> - 2001-04-30 09:39:06
|
It looks like the consensus is this: [1] We continue to document the format of the DPB. [2] We publish the restriction that we can only handle file names up to = 254 characters in length ( the length byte is 0..255 but includes the = trailing NUL byte ) [3] We remove isc_expand_dpb() from the published API, or depreciate it = as a "DO NOT USE" warning. I don't see any reason to mention it at all. > > 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 correct, this is indeed a problem waiting to crash and should be = logged. > /Mike I was somewhat in error. In Windows 98 the length of a file name is = limited to MAX_PATH characters, which is 260. In Windows NT/2000, using = Unicode, by prepending \\?\ to the name, the name can get to 32,000 = characters. Thus, not a panic, but down the road. Obviously these extremely long file names are not for users to type. = They will be generated. I have already seen a URL of more than 250 = characters. |
From: Ann W. H. <aha...@ib...> - 2001-04-30 14:06:48
|
At 04:38 PM 4/30/2001 +0700, Andy Canfield wrote: >[2] We publish the restriction that we can only handle file names up to >254 characters in length ( the length byte is 0..255 but includes the >trailing NUL byte ) It can, but the null byte is not required. >[3] We remove isc_expand_dpb() from the published API, or depreciate it as >a "DO NOT USE" warning. I don't see any reason to mention it at all. Deprecate it. I would prefer to see all entry points documented. Regards, Ann www.ibphoenix.com We have answers. |
From: reed m. <rf...@cr...> - 2001-04-30 17:21:56
|
Andy Canfield wrote: > > It looks like the consensus is this: > > [1] We continue to document the format of the DPB. > > [2] We publish the restriction that we can only handle file names up to = > 254 characters in length ( the length byte is 0..255 but includes the = > trailing NUL byte ) > > [3] We remove isc_expand_dpb() from the published API, or depreciate it = > as a "DO NOT USE" warning. I don't see any reason to mention it at all. > We need to keep it documented on principle. Even if it is just so that future generations will know why it is bad... IMHO, we should not have any 'undocumented' API's or features. Calling something undocumented is just a closed soure way of hiding warts or attempting to secure by obscurity. But we have some many warts that hiding them is futile (anyone care to drop firebird for WarthogSQL... I think I could come up with a nice ugly logo ;-) [...] -- Reed Mideke email: rfm(at)cruzers.com -If that fails: rfm(at)portalofevil.com |
From: Helen B. <he...@di...> - 2001-04-30 12:17:22
|
At 04:38 PM 30-04-01 +0700, you wrote: >It looks like the consensus is this: > > > >[3] We remove isc_expand_dpb() from the published API, or depreciate it = as a "DO NOT USE" warning. I don't see any reason to mention it at all. Huh? "Deprecate" means leave it there, document it and explain why it's = not a good idea to use it in new application code (and why it's a good id= ea to fix it if it's been used in existing code). Unless, of course, we = *want* Firebird to be incompatible with IBX...or any other existing appli= cation that uses it. Cheers, Helen All for Open and Open for All=20 InterBase Developer Initiative =B7 http://www.interbase2000.org _______________________________________________________ |
From: Ann W. H. <aha...@ib...> - 2001-04-29 16:05:04
|
At 01:55 PM 4/29/2001 +0700, Andy Canfield wrote: >What's wrong with isc_expand_dpb() - It was intended as a way for gpre to handle runtime dpb parameters. For reasons obscure, to me, someone decided it was a great thing for people writing api programs. It's not. The reasonable way for an api program to create a dpb is with malloc or isc_alloc. Create it at runtime, taking a stab at how long it should be. For multi-byte numerics, call isc_vax_integer. It does the right thing. If you find you've got more data than fits, do what you'd ordinarily do if a buffer was about to overflow. Call malloc (or isc_alloc) again, get a bigger buffer, copy over what you've got and release the old buffer. As for long file names, that's a problem. The rule for tpb's, dpb's, and info blocks is that the server looks at the token type; if it understands that type, it processes it. If not, the next byte tells it how many bytes to skip. Changing to a two-byte length requires a new version of all those structures (I'd guess) and will not be compatible with InterBase. My preference would be a declared restriction of 255 characters in a file name used by Firebird. Regards, Ann www.ibphoenix.com We have answers. |
From: Olivier M. <om...@ti...> - 2001-04-29 18:13:20
|
Dear Ann and all, On Sunday, April 29, 2001 (18:06), you wrote : >>What's wrong with isc_expand_dpb() - AWH> It was intended as a way for gpre to handle runtime AWH> dpb parameters. For reasons obscure, to me, someone AWH> decided it was a great thing for people writing api AWH> programs. It's not. As a heavy user of IB/FB through the API, I completely support the idea that the only thing wrong with isc_expand_dpb() is its existence. The only usefull fix would be to declare it "deprecated" and remove it in a later major release. There is no advantage to have such a function in the API. Best Regards, -- Olivier Mascia, om...@ti... TIP Group S.A., www.tipgroup.com "Les médecins passent leur vie à mettre des drogues qu'ils ne connaissent pas dans des corps qu'ils connaissent encore moins." - Voltaire -- |
From: Andy C. <an...@ad...> - 2001-05-04 04:54:01
|
>>[3] We remove isc_expand_dpb() from the published API,=20 > or depreciate it as a "DO NOT USE" warning.=20 > I don't see any reason to mention it at all. > Huh? "Deprecate" means leave it there,=20 OK; I guess I have a little stronger concept of "depreciate" than you = do. When I heard "depreciate", I didn't think of "twenty percent = discount", I thought of "Your girlfriend has a Y chromosome". = Fundamentally isc_expand_dpb() isn't what it pretends to be, and never = will be. So we document just exactly what it really does, and warn = people never to write a line of code that calls it, maybe even publish = the source code for an application-side function that does the job the = right way but can't be included in gds32.dll for compatability reasons.=20 AFAIK, - [1] The "length" parameter can NOT be the allocated length. If a new = buffer is allocated, this is the number of UCHARs that are copied from = the old buffer to the new buffer. It is also the target point where the = new stuff is added to the data. Therefore it MUST be the data length, = not the buffer size. Since the caller calls isc_expand_dpb() to add = stuff to the dpb, the required buffer length will always be greater than = the existing data length, so a new buffer is always allocated. [2] isc_expand_dpb() does not check to see if there is an item of this = type already in the buffer. If you have a user name and then call = isc_expand_dpb() with a user name, you'll get two user names in the = buffer. This might well confuse isc_attach_database(), so we should warn = on this, also. [3] I think that someone said you can only call isc_expand_dpb() once; = call it twice and it's all over. Also, does anybody know if = isc_expand_dpb() is thread safe? These questions depend on how = gds__alloc() works, which is what isc_expand_dpb() calls to get the new = buffer. To my ignorant eyes it's thread safe if and only if gds__alloc() = is. [4] The source code shows that isc_expand_dpb() calls gds_alloc() and a = comment says that gpre generates a call to gds_free(). But gds_free() is = not a documented part of the API. Are gds_alloc() and gds_free() on the = application side or the Firebird side of the API boundary? Either way, = if the comment is true then we have an undocumented bridge over the API = gap. The evaluation criteria for any code is correctness, robustness, = readability, and flexibility. Looking at the source code for = isc_expand_dpb(), I don't think much of it. COMPATABILITY ramble - I may be behind on this issue.=20 At run time, where does isc_expand_dpb() code reside - in the EXE or in = the DLL? Because we can add functionality to the EXE; there's no = (rational) issue of sombody compiling with one set of header files and = linking with a different set of library files. On the other hand, if = isc_expand_dpb() resides in the DLL, then there is an issue of what = happens when a new EXE encounters an old DLL, or a Firebird EXE = encounters an Interbase DLL. AFAIK, a Firebird application program when compiled creates calls into = gds32.dll. Previously I pictured a Firebird client machine ( application = and gds32.dll ) talking to an Interbase server machine, and vice versa. = In such a case it is the pipeline between the client machine and the = server machine which must be compatable. If there are extensions, the = gds32.dll must be prepared to test the server to see if it supports = those extensions and fail soft. Presumably, in the past, if you developed an application under Interbase = 6 there was no guarantee that your code would run properly if you had an = Interbase 5 gds32.dll installed on the client computer. True? Helen's comments imply that an application compiled and linked with = Firebird must be able to run with an Interbase gds32.dll, and = vice-versa. In that case neither Firebird nor Borland/Interbase can ever = make any change to the API. Even if we negotiated with Borland and = agreed on a change, the users would be confused as to which version of = Firebird ( 1.5? 2? 2.1? ) corresponded to which version of Interbase ( = 7? 8? 9? ). For example, suppose Firebird implemented a new connection string = protocol. But because the Firebird client application might be faced = with an InterBase gds32.dll which doesn't understand it, we dare not do = so. Nobody can change anything.=20 If we accept the principle that a Firebird client application must be = able to call an Interbase gds32.dll and vice versa, then the API can = never ever change; ever. True? Is that what we want? We could propogate the idea that a Firebird application requires a = Firebird gds32.dll, and an Interbase application requires an Interbase = gds32.dll. I think that the easiest way to ensure that is to rename our = DLL to something else. Then IB can call "gds32.dll" and FB can call = "gds32FB.DLL", both can be installed on one computer, both applications = can run, can talk through the DLL to the server (assuming network = pipeline compatability). What do we mean when we say "compatable"? Clearly you can't run an = Interbase 7 application using a gds32.dll from Interbase 6, yes? So why = be able to run an Interbase 7 application using a Firebird gds32.dll? |
From: Ann W. H. <aha...@ib...> - 2001-05-04 15:51:48
|
At 11:47 AM 5/4/2001 +0700, Andy Canfield wrote: >The evaluation criteria for any code is correctness, robustness, >readability, and flexibility. Looking at the source code for >isc_expand_dpb(), I don't think much of it. Efficiency fits in there somewhere, and the mix depends on the function. We should leave isc_expand_dpb in the documentation and mark it as a function generated by gpre and deprecate it's use elsewhere. >COMPATABILITY ramble - > >At run time, where does isc_expand_dpb() code reside - in the EXE or >in the DLL? That code, and other pieces of utility code exist in the client side library (gds32.dll to Gates weenies), and the server side library for classic, and probably the SuperServer exe (not completely sure about that). COMPATIBILITY lecture - The InterBase architecture was designed for upward compatibility. That's why the dpb (database parameter block) and tpb (transaction parameter block), and blr statement begin with a version number, and why the dbp, tpb, & info blocks are set up to allow a program to skip parameters it doesn't understand. Some history: The original InterBase API is a second implementation of DSRI - the Digital Standard Relational Interface. DSRI was largely created by Jim Starkey. It was extremely detailed and specific because DEC had two competing relational database projects and required that a program compiled and linked against one run, unchanged, against the other. An architectural process was set up between the two competing groups and a moderator was hired. He had two qualifications: a PhD in Computer Science from CMU, and experience as a bouncer in the Buckets of Blood Bar, outside the steel mill gates in Pittsburg. The first did him no good at all. The architecture that evolved from that process was very very good at hiding differences in implementation, philosophy, and standards of hygiene. Borland was not completely faithful to the architecture, leaving us with some problems. However, the goal should be that any version x client of either InterBase or Firebird can run against any server of version x or greater. To address the connection string issue. The connection parameters are set in the dpb. If we want to change what is sent, we create new parameters and InterBase ignores them. Unless, of course, they decide to use the same number for something else. With very moderate levels of cooperation, they won't. We could help by creating new parameters down from 254 rather than up from 0. Writing code that preserves compatibility and moves the product forward is challenging, but we're up to it. Cheers, Ann |
From: John B. <bel...@cs...> - 2001-05-04 16:04:44
|
This issue is important to me, because I have an outstanding dpb addition. I'm fairly certain that we don't have any established communication with Borland. Is it time to try (again) to set something up? I would feel more comfortable adding the dpb parameter is I knew that IB would not use the same number. -John On Friday, May 4, 2001, at 08:53 AM, Ann W. Harrison wrote: > COMPATIBILITY lecture - > > ....... > To address the connection string issue. The connection parameters > are set in the dpb. If we want to change what is sent, we create new > parameters and InterBase ignores them. Unless, of course, they decide > to use the same number for something else. With very moderate levels > of cooperation, they won't. We could help by creating new parameters > down from 254 rather than up from 0. > > Writing code that preserves compatibility and moves the product forward > is challenging, but we're up to it. > > Cheers, > > Ann > |
From: Helen B. <he...@di...> - 2001-05-04 16:20:52
|
At 09:04 AM 04-05-01 -0700, John Bellardo wrote: >This issue is important to me, because I have an outstanding dpb additio= n. I'm fairly certain that we don't have any established communication w= ith Borland. Is it time to try (again) to set something up? I would fee= l more comfortable adding the dpb parameter is I knew that IB would not u= se the same number. > >-John > >On Friday, May 4, 2001, at 08:53 AM, Ann W. Harrison wrote: > >>COMPATIBILITY lecture - >> >>....... >>To address the connection string issue. The connection parameters >>are set in the dpb. If we want to change what is sent, we create new >>parameters and InterBase ignores them. Unless, of course, they decide >>to use the same number for something else. With very moderate levels >>of cooperation, they won't. We could help by creating new parameters >>down from 254 rather than up from 0. Could be a good time to test Jon Arthur's assertion in last week's chat s= ession, that they want to work with the open source developers. Explicitly propose that Borland increment numbers from the bottom up and = FB decrement them from the top down. It should be to everybody's benefit= . Let Mark email Charlie and post a copy of the email on the FB main pag= e. Helen All for Open and Open for All=20 InterBase Developer Initiative =B7 http://www.interbase2000.org _______________________________________________________ |
From: John B. <bel...@cs...> - 2001-05-04 16:29:57
|
On Friday, May 4, 2001, at 09:15 AM, Helen Borrie wrote: > At 09:04 AM 04-05-01 -0700, John Bellardo wrote: >> This issue is important to me, because I have an outstanding dpb=20 >> addition. I'm fairly certain that we don't have any established=20 >> communication with Borland. Is it time to try (again) to set=20 >> something up? I would feel more comfortable adding the dpb parameter=20= >> is I knew that IB would not use the same number. >> >> -John >> >> On Friday, May 4, 2001, at 08:53 AM, Ann W. Harrison wrote: >> >>> COMPATIBILITY lecture - >>> >>> ....... >>> To address the connection string issue. The connection parameters >>> are set in the dpb. If we want to change what is sent, we create = new >>> parameters and InterBase ignores them. Unless, of course, they = decide >>> to use the same number for something else. With very moderate = levels >>> of cooperation, they won't. We could help by creating new = parameters >>> down from 254 rather than up from 0. > > Could be a good time to test Jon Arthur's assertion in last week's = chat=20 > session, that they want to work with the open source developers. > > Explicitly propose that Borland increment numbers from the bottom up=20= > and FB decrement them from the top down. It should be to everybody's=20= > benefit. Let Mark email Charlie and post a copy of the email on the = FB=20 > main page. > > Helen > > All for Open and Open for All > InterBase Developer Initiative =B7 http://www.interbase2000.org I'd send a copy to Jon Arthur directly (as well as to Charlie), and cc=20= firebird-devel and BPI-opensource (at least). If we want it on the main=20= web site we can reference the firebird-devel post. That way it will be=20= in public archives (other than just Firebird's). Hmm, sounds like a=20 plan to me, but I'm not one to bow away from having others do things.... One a side note, if Borland is willing to do this I suggest we reserve=20= our "first" code (254) as an expansion code to use sometime in the=20 future when our dpb space fills up :) -John |
From: Ann W. H. <aha...@ib...> - 2001-05-04 16:54:29
|
For once, I agree with everybody. Ask about getting number in the normal range and explain what you're doing with it. Then propose the "I'll keep on my side of the fence, you keep on yours" approach. And lets do this as publicly as we can. (And yes, reserve a number at the top of the range.) Regards, Ann |
From: Mike N. <ta...@al...> - 2001-05-04 16:32:37
|
Helen Borrie wrote: > Could be a good time to test Jon Arthur's assertion in last week's chat > session, that they want to work with the open source developers. This I agree with. If contact with this chap displays they indeed are willing to start a discussion/cooperation that's good. > Explicitly propose that Borland increment numbers from the bottom up > and FB decrement them from the top down. It should be to everybody's benefit. Ehh, shouldn't we as the first stop try to reserve the same address space (the same numbers) for the same features? If not, we're already as good as _completely_ alienated from eachother and all what that means. If that fails, then we could (as a very last resort I think) propose this scheme. /Mike |
From: John B. <bel...@cs...> - 2001-05-04 16:45:12
|
On Friday, May 4, 2001, at 09:32 AM, Mike Nordell wrote: > Helen Borrie wrote: > >> Explicitly propose that Borland increment numbers from the bottom up >> and FB decrement them from the top down. It should be to everybody's > benefit. > > Ehh, shouldn't we as the first stop try to reserve the same address > space > (the same numbers) for the same features? If not, we're already as good > as > _completely_ alienated from eachother and all what that means. > > If that fails, then we could (as a very last resort I think) propose > this > scheme. > > /Mike > I think nicely sharing the same address space is ideal. We should propose that first. Borland's response will show how willing they are to cooperate. Taking the time to agree on each parameter is, in my mind, more willing than "you get your half, we get ours". -John |