[DRMAA-WG] DRMAAv2 C binding - Final Draft
Bill Bryce
bbryce at univa.com
Sat May 12 07:28:48 EDT 2012
Good example Daniel. We don't have to do what POSIX does.
Regards
Bill
Sent from my iPad - US cell: 847-612-8966
On 2012-05-12, at 4:42 AM, Daniel Gruber <dgruber at univa.com> wrote:
> I wouldn't say that a programmer would be astonished with any
> solution. The signature implies *unambiguously* what is going to
> be changed. Of course an OS free() doesn't NULL because it
> can't. I wouldn't compare the DRMAA obj free with traditional raw free
> or OS defined functions, which most likely didn't change the last
> decades.
>
> In popular libraries like OpenMPI (which drmaa app writer most
> likely use) you have the exact counter-examples:
>
> "int MPI_Comm_free(MPI_Comm *comm)
>
> Description:
> This operation marks the communicator object for deallocation. The handle is set to MPI_COMM_NULL."
> (see http://www.open-mpi.org/doc/v1.4/man3/MPI_Comm_free.3.php)
>
> Same is with all other MPI object frees (with exception of a "raw" memory free).
>
> Source of their frees looks like this:
> 166 /*
> 167 * Free an info handle and all of its keys and values.
> 168 */
> 169 int ompi_info_free (ompi_info_t **info)
> 170 {
> 171 (*info)->i_freed = true;
> 172 OBJ_RELEASE(*info);
> 173 *info = MPI_INFO_NULL;
> 174 return MPI_SUCCESS;
> 175 }
> 176
> from: https://svn.open-mpi.org/source/xref/ompi-trunk/ompi/info/info.c
>
> Anyway both solutions would work, but until now I couldn't find any real disadvantage...
>
> Cheers,
>
> Daniel
>
>
>
> Am 11.05.2012 um 21:18 schrieb Peter Tröger:
>
>>> 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.
>>
>> Completely agreed - there is nothing like this in POSIX, which was the golden standard at least for DRMAAv1.
>>
>> Best regards,
>> Peter.
>>
>>
>>>
>>> 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
>>>
>>>
>> --
>> drmaa-wg mailing list
>> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.ogf.org/pipermail/drmaa-wg/attachments/20120512/7e7d753a/attachment-0001.html>
More information about the drmaa-wg
mailing list