Discussion:
[DRMAA-WG] DRMAAv2 C binding - Final Draft
Peter Tröger
2012-04-17 09:16:01 UTC
Permalink
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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: drmaa-c-finaldraft-annotated.pdf
Type: application/pdf
Size: 237571 bytes
Desc: not available
URL: <http://www.ogf.org/pipermail/drmaa-wg/attachments/20120417/bc3dd137/attachment-0002.pdf>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: drmaa-c-finaldraft-official.pdf
Type: application/pdf
Size: 207144 bytes
Desc: not available
URL: <http://www.ogf.org/pipermail/drmaa-wg/attachments/20120417/bc3dd137/attachment-0003.pdf>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: drmaa2.h
Type: application/octet-stream
Size: 18237 bytes
Desc: not available
URL: <http://www.ogf.org/pipermail/drmaa-wg/attachments/20120417/bc3dd137/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4343 bytes
Desc: not available
URL: <http://www.ogf.org/pipermail/drmaa-wg/attachments/20120417/bc3dd137/attachment-0001.bin>
Daniel Gruber
2012-05-09 14:47:50 UTC
Permalink
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
- 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. Hence I would recommend to have a drmaa2_string type because all other data (which has to be freed) is drmaa2 typed.

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 */
}

Thanks,

Daniel
Post by 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
https://www.ogf.org/mailman/listinfo/drmaa-wg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.ogf.org/pipermail/drmaa-wg/attachments/20120509/3a07a692/attachment.html>
Andre Merzky
2012-05-09 19:26:22 UTC
Permalink
Post by Daniel Gruber
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?
+1

Cheers, Andre.
Post by Daniel Gruber
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.
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 */
}
Thanks,
Daniel
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
?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
--
Nothing is ever easy...
Daniel Gruber
2012-05-11 13:17:45 UTC
Permalink
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
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 :)

Cheers,

Daniel
Thanks,
Daniel
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
Daniel Gruber
2012-05-11 14:51:58 UTC
Permalink
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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
Daniel Templeton
2012-05-11 16:47:37 UTC
Permalink
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
Post by Daniel Gruber
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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
Rayson Ho
2012-05-11 17:34:08 UTC
Permalink
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/
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
Post by Daniel Gruber
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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/
--
?drmaa-wg mailing list
?drmaa-wg at ogf.org
?https://www.ogf.org/mailman/listinfo/drmaa-wg
Bill Bryce
2012-05-11 18:28:59 UTC
Permalink
Hi Rayson,

Just to make sure I understand you. Do you mean..."It is more consistent not to NULL pointers" because C programmers typically know that they should handle this themselves?

Regards,

Bill.
Post by Rayson Ho
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/
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
Post by Daniel Gruber
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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/
--
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
William Bryce | VP of Products
Univa Corporation - 1001 Warrenville Road, Suite 100 Lisle, Il, 65032 USA
Email bbryce at univa.com | Mobile: 512.751.8014 | Office: 416.519.2934
Rayson Ho
2012-05-11 18:32:50 UTC
Permalink
Hi Bill,

Right, this is what C & C++ system libraries do - they don't touch the
pointer variable passed into them.

Examples: realloc(), free() - the pointer is just a variable that
tells the library routine where to go to get the memory, but the
pointer variable itself should not be changed by the library routine.

Rayson

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

Scalable Grid Engine Support Program
http://www.scalablelogic.com/
Post by Bill Bryce
Hi Rayson,
Just to make sure I understand you. ?Do you mean..."It is more consistent not to NULL pointers" because C programmers typically know that they should handle this themselves?
Regards,
Bill.
Post by Rayson Ho
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/
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
Post by Daniel Gruber
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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/
--
?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
William Bryce | VP of Products
Univa Corporation - 1001 Warrenville Road, Suite 100 Lisle, Il, 65032 USA
Email bbryce at univa.com | Mobile: 512.751.8014 | Office: 416.519.2934
Peter Tröger
2012-05-11 19:18:06 UTC
Permalink
Post by Daniel Templeton
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.
Post by Daniel Templeton
Daniel
Post by Daniel Gruber
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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
Daniel Gruber
2012-05-12 08:42:21 UTC
Permalink
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
Post by Daniel Templeton
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.
Post by Daniel Templeton
Daniel
Post by Daniel Gruber
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.ogf.org/pipermail/drmaa-wg/attachments/20120512/bd4413f9/attachment-0001.html>
Bill Bryce
2012-05-12 11:28:48 UTC
Permalink
Good example Daniel. We don't have to do what POSIX does.

Regards

Bill

Sent from my iPad - US cell: 847-612-8966
Post by Daniel Gruber
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
"int MPI_Comm_free(MPI_Comm *comm)
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).
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
Post by Daniel Templeton
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.
Post by Daniel Templeton
Daniel
Post by Daniel Gruber
Hi Daniel
Post by Daniel Gruber
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
Post by Daniel Gruber
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 ;)
Post by Daniel Gruber
- 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.
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.)
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
Post by Daniel Gruber
Cheers,
Daniel
Thanks,
Daniel
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>
Rayson Ho
2012-05-12 14:50:25 UTC
Permalink
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/
Post by Bill Bryce
Good example Daniel. We don't have to do what POSIX does.
Regards
Bill
Sent from my iPad - US cell: 847-612-8966
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
"int MPI_Comm_free(MPI_Comm *comm)
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).
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
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
Hi Daniel
Hi Stefan,
Hello,
I put my comments below.
Best regards
Stefan
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.
//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.
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.)
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
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
Loading...