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