[m-dev.] for review: bootstrap hlc.gc.memprof grade

Peter Ross peter.ross at miscrit.be
Thu Aug 10 18:37:57 AEST 2000


On Thu, Aug 10, 2000 at 06:23:07AM +1000, Fergus Henderson wrote:
> On 09-Aug-2000, Peter Ross <peter.ross at miscrit.be> wrote:
> > On Tue, Aug 08, 2000 at 06:14:57PM +1000, Fergus Henderson wrote:
> > > On 02-Aug-2000, Peter Ross <peter.ross at miscrit.be> wrote:
> > > > runtime/mercury_prof.c:
> > > > runtime/mercury_prof.h:
> > > >     Make decl_fptr an extern global pointer so that mercury_wrapper.c
> > > >     can call fclose on it.
> > > > 
> > > > runtime/mercury_wrapper.c:
> > > >     Call fclose on decl_fptr.
> > > 
> > > Why do you need to call fclose(decl_ptr) from mercury_wrapper.c?
> > > There's already code to fclose(decl_ptr) in the MR_prof_finish()
> > > function in runtime/mercury_prof.c.  Isn't that the right place
> > > to do the close?
> > > 
> > If you examine the file under WinNT before the file has been closed, bad
> > things happen!  So I changed to code so that fclose happened as soon as
> > possible, unlike MR_prof_finish() which is called at the end of
> > execution.
> 
> Hmm, I think this change may cause more problems than it solves.
> In particular:
> 	- you didn't check the return value from fclose();
> 	  probably you should be calling checked_fclose() rather than fclose().
> 	- you've added a call to MR_fclose(),
> 	  but you didn't remove the old call to MR_fclose(),
> 	  so the file will get closed twice.  This will lead to
> 	  undefined behaviour...
> 	- if do_init_modules() is called multiple times, then
> 	  the file will get closed multiple times, again leading
> 	  to undefined behaviour.
> 
> You need to fix those problems.
> 
> There's also another minor issue: unlike the code in mercury_prof.c,
> you didn't check to see whether decl_fptr is null.  That may be OK I
> guess, but it might be safer to check it.
> 
> I think it would be better to abstract things a little, and do
> the work from some function in mercury_prof.c which gets called
> by mercury_wrapper.c, rather than doing it directly from mercury_wrapper.c.
> 
OK will do.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------



More information about the developers mailing list