[DRMAA-WG] DRMAAv2 C binding - Final Draft

Fri May 11 13:34:08 EDT 2012

Hi Dan - welcome back!

I also think that it is more consistent to not store NULL a pointer.

That interface is way easier for normal programmers to understand as
the most common memory allocation routines are malloc() & free().

Rayson

=================================
Open Grid Scheduler / Grid Engine
http://gridscheduler.sourceforge.net/

Scalable Grid Engine Support Program
http://www.scalablelogic.com/

On Fri, May 11, 2012 at 12:47 PM, Daniel Templeton <daniel at cloudera.com> wrote:
> The main argument against NULLing the pointer is really just principle of
> least astonishment -- routines in C don't typically do that, especially not
> in OS libraries.  Because there's such a large body of function calls that
> don't do it, throwing in some that do, can make your code confusing.  Did
> you forget to NULL that pointer, or was it done in the library?  NULLing the
> pointer is more practical, but only if everyone does it that way.  I would
> vote to conform to what the majority of libraries already do.
>
> (Yes, I really did just respond to a DRMAA email!)
>
> Daniel
>
>
> On 5/11/12 7:51 AM, Daniel Gruber wrote:
>>
>> 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
>>>>>
>>>>>
>> --
>>   drmaa-wg mailing list
>>   drmaa-wg at ogf.org
>>   https://www.ogf.org/mailman/listinfo/drmaa-wg
>
>
>
> --
> Get Apache Hadoop for the Enterprise: http://www.cloudera.com/downloads/
> Follow us @cloudera or http://www.facebook.com/cloudera
>
>
> --
>  drmaa-wg mailing list
>  drmaa-wg at ogf.org
>  https://www.ogf.org/mailman/listinfo/drmaa-wg