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
[email protected]<mailto:[email protected]>
https://www.ogf.org/mailman/listinfo/drmaa-wg
--
drmaa-wg mailing list
[email protected]
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
[email protected]
https://www.ogf.org/mailman/listinfo/drmaa-wg
