Am 11.05.2012 um 16:27 schrieb Klauck, Stefan: > Hi Daniel > > > On May 11, 2012, at 3:17 PM, Daniel Gruber wrote: > >> Hi Stefan, >> >> Am 11.05.2012 um 14:49 schrieb Klauck, Stefan: >> >>> Hello, >>> >>> I put my comments below. >>> >>> Best regards >>> Stefan >>> >>> >>> On May 9, 2012, at 4:47 PM, Daniel Gruber wrote: >>> >>> For the current public comment period I've following issues: >>> >>> 295: drmaa2_list_get(.., int pos) -> pos should be const, the >>> implementation does not need to change it >>> - 297 int pos -> same >>> >>> The 'const' is not really necessary in the header file since the >>> implementation file can add the const, too. >>> >>> The following example is legal c code: >>> >>> //foo.h >>> void foo(int bar); >>> >>> //foo.c >>> void foo(const int bar); >>> >> >> But you will loose the advantage of pushing a const into the function. > > Do you mean that you do not force the implementation to use a const?
Now I'm lost :) Yes, I want to have the implementations that they are forced to use the const. Hence I wan't to have this in the spec like the other consts there. This would allow DRMAA application implementers to pass a const. Having const in the innermost functions is always a good idea in order to avoid bad casts. > >> >> You could also avoid "bar" in the header (like void foo(int)), but this makes >> code hard to maintain. >> Since we have already "const" in the spec, in my >> eyes it would look in my eyes more consistent. >> > > I agree. My point was that it is not really necessary to do not allow the > implementation to change the 'pos' value since the value is not returned > anyway. > Nevertheless, the implementation could add the 'const' for optimization… You're right, now we can force them to do so ;) > >>> >>> >>> - 549 char **substate -> the user has to free it with >>> "drmaa2_string_free()". Since it is just a string the user might "forget" >>> this and use >>> a "normal" free. >>> >>> In the current mock implementation >>> https://github.com/troeger/drmaav2-mock/blob/master/drmaa2.c >>> drmaa2_string_free() is a normal free(), so that this would be ok (except >>> for the NULL'ing). >> >> Indeed. Here it wouldn't be a problem. >> >>> >>> Hence I would recommend to have a drmaa2_string type because all other data >>> (which has to be freed) is drmaa2 typed. >>> >>> >>> Sounds nevertheless reasonable. >>> >>> >>> General question: I would like to have the free functions to NULL the freed >>> pointers (in order to minimize the risk with >>> dangling pointers). Hence I would change *all* free methods to accept ** >>> arguments. Wouldn't this be reasonable? >>> Why should the user always do the NULL'ing itself? He might forget or be >>> just lazy and running later in problems, which >>> we could avoid easily. >>> >>> Example: >>> drmaa2_list_free(&list); >>> /* some code later, I want to do something with list again, but not sure if >>> it was freed already */ >>> ... >>> if (list == NULL) { >>> /* oh it was freed already */ >>> } else { >>> /* not feed */ >>> } >>> >>> >>> I had no opinion. Hence I googled the problem. >>> There are many conversations concerning this topic especially on >>> stackoverflow >>> e.g. >>> http://stackoverflow.com/questions/1879550/should-one-really-set-pointers-to-null-after-freeing-them >>> . >>> >>> Since the "NULL'ing" is controversial, I suggest to do not set the pointer >>> to NULL within the implementation. >>> Applications can still implement macros or wrapper functions (for NULL'ing) >>> to write portable code, in case they want to set freed pointers to NULL and >>> rely on that. >> >> Can't see any valid argument against, sorry could read the full article in >> all details. >> I guess we've to vote :) > > You lose the address of the freed pointer. (That's why I would let the > application developer to decide.) > > example: > > drmaa2_list list_reference = list; > drmaa2_list_free(&list); > if (list == list_reference) // false Uhh, wasn't aware of this case (sorry, my mistake I wanted to say "couldn't read the full article in all details)...). Is such a reference to an pointer we've really needed in DRMAA apps? I'm wondering what would s.o. could do with this reference pointing to some (most likely) bad memory location. IMHO using a pointer after freeing is nothing s.o. should do by choice. Hence immediately after the list_free() the reference (if it is really needed) should be NULLed as well. Anybody who really wants to do that? Roger? Andre? Cheers, Daniel > > Even though list and list_refernce are invalid pointers, it is not really > intuitive that the comparison evaluates to 'false'. > > In addition "if (list_reference == NULL)" is still 'false' > > > > Cheers > Stefan > >> >> Cheers, >> >> Daniel >> >>> >>> Thanks, >>> >>> Daniel >>> >>> >>> Am 17.04.2012 um 11:16 schrieb Peter Tröger: >>> >>> Dear all, >>> >>> the DRMAAv2 C binding is now in final draft state. Attached you can find >>> the annotated and the official version of the specification, as well as the >>> raw header file. >>> >>> Please provide your final comments on the mailing list until *April 22nd* >>> (this Sunday). In case, we will set up a conf call for last discussions. >>> Otherwise, the document will enter the OGF document process on next Monday. >>> >>> Thanks and best regards, >>> Peter. >>> >>> >>> <drmaa-c-finaldraft-annotated.pdf><drmaa-c-finaldraft-official.pdf><drmaa2.h>-- >>> drmaa-wg mailing list >>> [email protected]<mailto:[email protected]> >>> https://www.ogf.org/mailman/listinfo/drmaa-wg >>> >>> >> > -- drmaa-wg mailing list [email protected] https://www.ogf.org/mailman/listinfo/drmaa-wg
