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