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. 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.
- 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... .
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 :) 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 drmaa-wg@ogf.org<mailto:drmaa-wg@ogf.org> https://www.ogf.org/mailman/listinfo/drmaa-wg