[DRMAA-WG] DRMAAv2 C binding - Final Draft

[Rayson Ho]

In the MPI case, you have a matching pair of functions - they both
take the address to the pointer (ie. &comm ) as the argument:

- allocate: MPI_Comm_dup( MPI_COMM_WORLD, &comm );
- free: MPI_Comm_free( &comm );

The difference is that in the DRMAA2 case you get the pointer to the
list from the return value of drmaa2_list_create(), and this is the
interface used by malloc(). Thus to stay consistent with the most
common dynamic memory interface, we should just use the same interface
for the free routine.

On the other hand, if we were to change the interface of
drmaa2_list_create() such that it requires the programmer to pass in
the address of a variable to store the return list, then I would agree
that we switch to the interface suggested by Daniel Gruber.

(I should have reviewed the draft so long ago, but if it wasn't DanT's
email, I probably wouldn't have enough motivation to read the whole
thing.)

Rayson

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

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



On Sat, May 12, 2012 at 7:28 AM, Bill Bryce <bbryce at univa.com> wrote:
> 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
>
>
> --
>  drmaa-wg mailing list
>  drmaa-wg at ogf.org
>  https://www.ogf.org/mailman/listinfo/drmaa-wg