[DRMAA-WG] DRMAAv2 C binding - Final Draft

[Daniel Gruber]

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