[m-rev.] For review: Make improvements to stack segments code.
Julien Fischer
juliensf at csse.unimelb.edu.au
Mon Jun 7 00:41:07 AEST 2010
Hi Paul,
Here are some initial comments on this change.
On Thu, 27 May 2010, Paul Bone wrote:
> Make improvements to stack segments code.
>
> The main benefits of these changes are:
>
> Stack segments (and other memory zones) are cached when they are released
> and can be re-used.
>
> Some thread softy-fixes have been added.
s/softy/safety/
> All stack segments are now the same size, including the first segment.
> The first segment no-longer has a redzone.
>
> Hard zones on all memory zones have been set to the minimum of one page
> rather than one MR_unit which is usually two pages.
>
> Other changes include corrections in code comments, clearer function names and
> a documentation fix.
>
>
> runtime/mercury_memory_zones.h:
> runtime/mercury_memory_zones.c:
> Re-write a lot of the code that managed the zone lists. The old code did
> not re-used previously allocated but saved zones. The changes ensure that
s/re-used/re-use/
> MR_create_or_reuse_zone (formerly MR_create_zone) checks for a free zone
> of at least the required size before allocating a new one. When zones are
> released they are put on the free list. Garbage collection of free zones
> is not yet implemented.
Will the garbage collector still scan the memory zones on the free list?
(Not free the zones could be problematic in trail segment grades because
if part of the program does something that allocates a lot of trail
segments, we may end up with a lot of memory zones sitting on the free
list after the commit; this may tie up memory that may be needed by
later parts of the program.)
> As above MR_create_zone is now MR_create_or_reuse_zone,
>
> MR_unget_zone is now MR_release_zone.
>
> MR_construct_zone has been removed.
Say why, i.e. it is no longer required.
>
> Not all zones are added to the used list for the benifit of the memory
s/benifit/benefit/
> error handlers.
>
> Do better locking on the free_memory_zones and used_memory_zones lists.
Better locking in what sense?
> Make sure that list modification makes memory writes in a clear order so
> that the lists are always well formed.
>
> Rename MR_get_used_memory_zones() to MR_get_used_memory_zones_readonly()
> and document that the zone lists may be incomplete.
>
> Make the MR_zone_next field of the MR_MemoryZone_Struct structure volatile.
>
> Remove MAX_ZONES, it wasn't being used anywhere.
>
> Insert some calls to MR_debug_log_message to help with debugging.
>
> runtime/mercury_stacks.c:
> Conform to changes in mercury_memory_zones.c.
>
> Use MR_GC_NEW rather than MR_GC_malloc_uncollectable for the list or
s/or/of/
> previous stack segments associated with a context. The context is heap
> allocated so the list need not be uncollectable.
>
> Use MR_debug_log_message for printf-style debugging rather than printf.
>
> runtime/mercury_wrapper.h:
> runtime/mercury_wrapper.c:
> Remove support for the smaller sized stacks in grades with stack segments.
>
> Disable redzones when using stack segments. The settings here apply to
> the first segment on every stack.
>
> Conform to changes in runtime/mercury_memory_zones.h.
>
> runtime/mercury_context.h:
> runtime/mercury_context.c:
> Removed an extra declaration for MR_init_context_maybe_generator
>
> Conform to changes in runtime/mercury_memory_zones.h.
> Conform to changes in runtime/mercury_wrapper.h.
>
> runtime/mercury_memory.c:
> Adjust the defintion of MR_unit. It is now gaurenteed to be a multiple of
> the page size which is required by it's use in mercury_memory_zones.c
>
> Conform to changes in mercury_wrapper.h.
>
> runtime/mercury_engine.c:
> runtime/mercury_memory_handlers.c:
> runtime/mercury_trail.c:
> Conform to changes in runtime/mercury_memory_zones.h.
>
> runtime/mercury_misc.c:
> Print out the meaning of errno if it is nonzero in MR_fatal_error.
>
> runtime/mercury_atomic_ops.h:
> Define MR_THREADSAFE_VOLATILE to expand to volatile when MR_THREADSAFE is
> defined. Otherwise it expands to nothing.
...
> Index: runtime/mercury_memory_zones.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_memory_zones.c,v
> retrieving revision 1.36
> diff -u -p -b -r1.36 mercury_memory_zones.c
> --- runtime/mercury_memory_zones.c 13 Feb 2010 07:29:10 -0000 1.36
> +++ runtime/mercury_memory_zones.c 27 May 2010 01:31:04 -0000
> @@ -63,8 +63,14 @@
>
> #ifdef MR_THREAD_SAFE
> #include "mercury_thread.h"
> + #include "mercury_atomic_ops.h"
> #endif
>
> +#include "mercury_memory_handlers.h"
> +
> +/*
> +** Why is this included here and not above with the other system includes?
> +*/
Did you leave out an XXX there?
> #ifdef MR_WIN32_VIRTUAL_ALLOC
> #include <windows.h>
> #endif
> @@ -339,19 +345,25 @@ MR_dealloc_zone_memory(void *base, size_
>
> /*---------------------------------------------------------------------------*/
>
> -#define MAX_ZONES 16
> +static void MR_init_offsets(void);
>
> -/* Enables a workaround in MR_unget_zone(). */
> -#define MEMORY_ZONE_FREELIST_WORKAROUND 1
> +static MR_MemoryZone* MR_get_free_zone(size_t size);
> +static void MR_add_zone_to_used_list(MR_MemoryZone *zone);
> +static void MR_remove_zone_from_used_list(MR_MemoryZone *zone);
> +static void MR_return_zone_to_free_list(MR_MemoryZone *zone);
> +/*
> +static void MR_gc_zones();
> +*/
> +static void MR_free_zone(MR_MemoryZone *zone);
> +static size_t get_zone_alloc_size(MR_MemoryZone *zone);
>
> -static MR_MemoryZone *used_memory_zones = NULL;
> -static MR_MemoryZone *free_memory_zones = NULL;
> -#ifdef MR_THREAD_SAFE
> - static MercuryLock free_memory_zones_lock;
> +#ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
> +static void
> +MR_configure_redzone_size(MR_MemoryZone *zone, size_t new_redsize);
> #endif
>
> -static void MR_init_offsets(void);
> -static MR_MemoryZone *MR_get_zone(void);
> +static MR_MemoryZone *
> +MR_create_new_zone(size_t desired_size, size_t redzone_size);
>
> /*
> ** We manage the handing out of offsets through the cache by
> @@ -367,11 +379,42 @@ static size_t *offset_vector;
> static int offset_counter;
> extern size_t next_offset(void);
>
> +
> +MR_THREADSAFE_VOLATILE MR_Unsigned MR_context_id_counter = 0;
> +
> +/*
> +** This list contains used zones that need a signal handler, for example those
> +** that have redzones. Other used zones may exist that arn't on this list
> +** because:
> +**
> +** 1) They don't have a readzone.
s/readzone/redzone/
(This occurs in several spots throughout this diff)
> +**
> +** 2) Putting them on this list in a threadsafe grade requires extra
> +** synchronisation.
> +*/
> +static MR_MemoryZone * volatile used_memory_zones = NULL;
> +static MR_MemoryZonesFree * volatile free_memory_zones = NULL;
>
> +#ifdef MR_THREAD_SAFE
> + /*
> + ** You must take this lock to write to either of the zone lists. Or to read
> + ** the complete zone lists.
That should be one sentence.
> Reading a zone list without taking the lock is
> + ** also supported _iff_ partial information is okay. Ane code that writes
s/Ane code/Any code/
> + ** the list must gaurentee that memory writes occur in the correct order so
s/gaurentee/guarantee/
> + ** that the list is always well formed from the POV of a reader.
> + **
> + ** This is necessary so that signal handlers can read the list without taking
> + ** a lock. They may not take a lock because pthread_mutex_lock cannot be
> + ** used safely within a signal handler.
> + */
> + static MercuryLock memory_zones_lock;
> +#endif
> +
> +
> void
> MR_init_zones()
> {
> #ifdef MR_THREAD_SAFE
> - pthread_mutex_init(&free_memory_zones_lock, MR_MUTEX_ATTR);
> + pthread_mutex_init(&memory_zones_lock, MR_MUTEX_ATTR);
> #endif
>
> MR_init_offsets();
> @@ -397,54 +440,94 @@ MR_init_offsets()
> }
>
> static MR_MemoryZone *
> -MR_get_zone(void)
> +MR_get_free_zone(size_t size)
> {
> MR_MemoryZone *zone;
> + MR_MemoryZonesFree *zones_list;
> + MR_MemoryZonesFree *zones_list_prev;
>
> /*
> ** Unlink the first zone on the free-list, link it onto the used-list
> ** and return it.
> */
> - MR_LOCK(&free_memory_zones_lock, "get_zone");
> - if (free_memory_zones == NULL) {
> - zone = MR_GC_NEW(MR_MemoryZone);
> + MR_LOCK(&memory_zones_lock, "get_zone");
s/get_zone/get_free_zone/ for the second argument of MR_LOCK there.
(Quite a few other instances of MR_LOCK and MR_UNLOCK now have incorrect
second arguments throughout this diff to renaming.)
> +
> + zones_list = free_memory_zones;
> + zones_list_prev = NULL;
> + while (zones_list) {
> + if (zones_list->MR_zonesfree_size <= size) {
> + /*
> + * A zone on this list will fit our needs.
> + */
> + break;
> + }
> + zones_list_prev = zones_list;
> + zones_list = zones_list->MR_zonesfree_tail;
> + }
> +
> + if (zones_list) {
> + zone = zones_list->MR_zonesfree_head;
> + if (zone->MR_zone_next) {
> + zones_list->MR_zonesfree_head = zone->MR_zone_next;
> } else {
> - zone = free_memory_zones;
> - free_memory_zones = free_memory_zones->MR_zone_next;
> + /*
> + ** This minor list is now empty, we should remove it from the major
> + ** list.
> + */
> + if (zones_list_prev) {
> + zones_list_prev->MR_zonesfree_tail = zones_list->MR_zonesfree_tail;
> + } else {
> + free_memory_zones = zones_list->MR_zonesfree_tail;
> + }
> + }
> + } else {
> + zone = NULL;
> }
>
> - zone->MR_zone_next = used_memory_zones;
> - used_memory_zones = zone;
> - MR_UNLOCK(&free_memory_zones_lock, "get_zone");
> + MR_UNLOCK(&memory_zones_lock, "get_zone");
>
> return zone;
> }
>
> -void
> -MR_unget_zone(MR_MemoryZone *zone)
> +static void
> +MR_add_zone_to_used_list(MR_MemoryZone *zone)
> {
> -#ifdef MEMORY_ZONE_FREELIST_WORKAROUND
> + MR_LOCK(&memory_zones_lock, "get_zone");
s/get_zone/unget_zone/
> +
> + zone->MR_zone_next = used_memory_zones;
> /*
> - ** XXX MR_construct_zone() does not yet reuse previously allocated memory
> - ** zones properly and simply leaks memory when it tries to do so. As a
> - ** workaround, we never put memory zones on the free list and deallocate
> - ** them immediately here.
> + ** This change must occur before we replace the head of the list.
> */
> - #ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
> +#ifdef MR_THREAD_SAFE
> + MR_CPU_SFENCE;
> +#endif
> + used_memory_zones = zone;
> +
> + MR_UNLOCK(&memory_zones_lock, "get_zone");
> +}
> +
> +static void
> +MR_free_zone(MR_MemoryZone *zone)
> +{
> +#ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
> size_t redsize;
> int res;
>
> redsize = zone->MR_zone_redzone_size;
> res = MR_protect_pages((char *) zone->MR_zone_redzone,
> - redsize + MR_unit, NORMAL_PROT);
> - assert(res == 0);
> - #endif
> + redsize + MR_page_size, NORMAL_PROT);
> + if (res) {
> + MR_fatal_error("Couldn't unprotect memory pages in MR_free_zone");
s/Couldn't/Could not/
> + }
> +#endif
>
> MR_dealloc_zone_memory(zone->MR_zone_bottom,
> ((char *) zone->MR_zone_top) - ((char *) zone->MR_zone_bottom));
> +}
>
> -#else /* !MEMORY_ZONE_FREELIST_WORKAROUND */
> -
> +static void
> +MR_remove_zone_from_used_list(MR_MemoryZone *zone)
> +{
> MR_MemoryZone *prev;
> MR_MemoryZone *tmp;
>
> @@ -453,11 +536,12 @@ MR_unget_zone(MR_MemoryZone *zone)
> ** then link it onto the start of the free-list.
> */
>
> - MR_LOCK(&free_memory_zones_lock, "unget_zone");
> - for(prev = NULL, tmp = used_memory_zones; tmp != NULL && tmp != zone;
> - prev = tmp, tmp = tmp->MR_zone_next)
> - {
> - /* VOID */
> + MR_LOCK(&memory_zones_lock, "remove_zone_from_used_list");
> + prev = NULL;
> + tmp = used_memory_zones;
> + while (tmp != NULL && tmp != zone) {
> + prev = tmp;
> + tmp = tmp->MR_zone_next;
> }
>
> if (tmp == NULL) {
> @@ -469,12 +553,92 @@ MR_unget_zone(MR_MemoryZone *zone)
> } else {
> prev->MR_zone_next = tmp->MR_zone_next;
> }
> + MR_UNLOCK(&memory_zones_lock, "remove_zone_frm_used_list");
> +}
> +
> +static void
> +MR_return_zone_to_free_list(MR_MemoryZone *zone)
> +{
> + MR_MemoryZonesFree *tmp_list;
Please use a more meaningful name than tmp_list.
> + size_t size;
> +
> + size = get_zone_alloc_size(zone);
> +
> + MR_LOCK(&memory_zones_lock, "ireturn_zone_to_free_list");
ireturn?
> +
> + tmp_list = free_memory_zones;
> + while (tmp_list) {
> + if (tmp_list->MR_zonesfree_size == size)
> + {
> + /*
> + ** We found the correct zone list.
> + */
> + break;
> + }
> + /*
> + ** Test to see if we can exit the loop early.
> + */
> + else if (tmp_list->MR_zonesfree_size > size)
> + {
> + /*
> + ** Set this to null to represent our failure to find a zone list of
> + ** the right size.
> + */
> + tmp_list = NULL;
> + break;
> + }
> + tmp_list = tmp_list->MR_zonesfree_tail;
> + }
>
> - zone->MR_zone_next = free_memory_zones;
> - free_memory_zones = zone;
> - MR_UNLOCK(&free_memory_zones_lock, "unget_zone");
> + if (tmp_list == NULL) {
> + MR_MemoryZonesFree *new_list, *prev_list;
> +
> + new_list = MR_GC_NEW(MR_MemoryZonesFree);
> + new_list->MR_zonesfree_size = size;
> + new_list->MR_zonesfree_head = NULL;
> + tmp_list = free_memory_zones;
> + prev_list = NULL;
> + while (tmp_list) {
> + if (tmp_list->MR_zonesfree_size > size) {
> + /*
> + ** We've just passed the position where this list item belongs.
> + */
> + break;
> + }
> + prev_list = tmp_list;
> + tmp_list = tmp_list->MR_zonesfree_tail;
> + }
> + /*
> + ** Insert it between prev_list and tmp_list.
> + */
> + new_list->MR_zonesfree_tail = tmp_list;
> + if (prev_list) {
> + prev_list->MR_zonesfree_tail = new_list;
> + } else {
> + free_memory_zones = new_list;
> + }
>
> -#endif /* MEMORY_ZONE_FREELIST_WORKAROUND */
> + /*
> + ** Reset tmp_list so that it is pointing at the correct magor list item
s/magor/major/
> + ** regardless of whether this branch was executed or not.
> + */
> + tmp_list = new_list;
> + }
> +
> + zone->MR_zone_next = tmp_list->MR_zonesfree_head;
> + tmp_list->MR_zonesfree_head = zone;
> +
> + MR_UNLOCK(&memory_zones_lock, "ireturn_zone_to_free_list");
> +}
...
> @@ -501,71 +665,109 @@ MR_next_offset(void)
> }
>
> MR_MemoryZone *
> -MR_create_zone(const char *name, int id, size_t size, size_t offset,
> +MR_create_or_reuse_zone(const char *name, size_t size, size_t offset,
> size_t redsize, MR_ZoneHandler handler)
> {
> MR_Word *base;
> size_t total_size;
> + MR_MemoryZone *zone;
> + MR_bool is_new_zone;
>
> + zone = MR_get_free_zone(size + redsize);
> + if (zone) {
> +#ifdef MR_DEBUG_STACK_SEGMENTS
> + MR_debug_log_message("re-using existing zone");
> +#endif
> + is_new_zone = MR_FALSE;
> + zone->MR_zone_desired_size = size;
> + } else {
> +#ifdef MR_DEBUG_STACK_SEGMENTS
> + MR_debug_log_message("allocating new zone");
> +#endif
> + is_new_zone = MR_TRUE;
> + zone = MR_create_new_zone(size, redsize);
> + }
> +
> + zone->MR_zone_name = name;
> +#ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
> + zone->MR_zone_handler = handler;
> + if (!is_new_zone && (redsize != zone->MR_zone_redzone_size)) {
> /*
> - ** total allocation is:
> - ** unit (roundup to page boundary)
> - ** size (including redzone)
> - ** unit (an extra page for protection if mprotect is being used)
> + ** The redzone must be reconfigured.
> */
> -#ifdef MR_PROTECTPAGE
> - total_size = size + 2 * MR_unit;
> + MR_configure_redzone_size(zone, redsize);
> + MR_reset_redzone(zone);
> + }
> #else
> - total_size = size + MR_unit;
> + if (!is_new_zone) {
> + zone->MR_zone_redzone_size = redsize;
> + }
> #endif
>
> - base = (MR_Word *) MR_alloc_zone_memory(total_size);
> - if (base == NULL) {
> - char buf[2560];
> - sprintf(buf, "unable allocate memory zone: %s#%d", name, id);
> - MR_fatal_error(buf);
> + if (redsize || (handler != MR_null_handler)) {
> + /*
> + ** Any zone with a redzone or a non-default handler must be
> + ** added the the used zones list.
> + */
> + MR_add_zone_to_used_list(zone);
> }
>
> - return MR_construct_zone(name, id, base, size, offset, redsize, handler);
> + return zone;
> }
>
> -MR_MemoryZone *
> -MR_construct_zone(const char *name, int id, MR_Word *base,
> - size_t size, size_t offset, size_t redsize, MR_ZoneHandler handler)
> +static MR_MemoryZone *
> +MR_create_new_zone(size_t desired_size, size_t redzone_size)
> {
> + size_t offset;
> MR_MemoryZone *zone;
> + MR_Word *base;
> size_t total_size;
>
> + offset = MR_next_offset();
> + /*
> + ** Ignore the offset if it is at least half the desired size of the zone.
> + ** This should only happen for very small zones.
> + */
> + if ((offset * 2) > desired_size) {
> + offset = 0;
> + }
> +
> + /*
> + ** The redzone must be page aligned and page multiple.
Do you mean:
... and its size must be a multiple of the page size.
> + */
> + redzone_size = MR_round_up(redzone_size, MR_page_size);
> + /*
> + ** Include an extra page size for the hardzone.
> + */
> + total_size = desired_size + redzone_size + MR_page_size;
> + /*
> + ** The total size must also be rounded to a page boundary, so that it can
> + ** be allocated from mmap if we're using accurate GC.
> + */
> + total_size = MR_round_up(total_size, MR_page_size);
> +
> + base = (MR_Word *) MR_alloc_zone_memory(total_size);
> if (base == NULL) {
> - MR_fatal_error("MR_construct_zone called with NULL pointer");
> + MR_fatal_error("Unable to allocate memory for zone");
> }
>
> - zone = MR_get_zone();
> + zone = MR_GC_NEW(MR_MemoryZone);
>
> - zone->MR_zone_name = name;
> - zone->MR_zone_id = id;
> - zone->MR_zone_desired_size = size;
> - zone->MR_zone_redzone_size = redsize;
> + zone->MR_zone_name = NULL;
> +#ifdef MR_THREAD_SAFE
> + zone->MR_zone_id = MR_atomic_add_and_fetch_uint(&MR_context_id_counter, 1);
> +#else
> + zone->MR_zone_id = ++MR_context_id_counter;
> +#endif
> + zone->MR_zone_desired_size = desired_size;
> + zone->MR_zone_redzone_size = redzone_size;
>
> #ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
> - zone->MR_zone_handler = handler;
> + zone->MR_zone_handler = NULL;
> #endif /* MR_CHECK_OVERFLOW_VIA_MPROTECT */
>
> - /*
> - ** XXX If `zone' is pulled off the free-list (rather than newly allocated)
> - ** then zone->MR_zone_bottom will be pointing to a previously allocated
> - ** memory region. Setting it to point to `base' therefore causes a memory
> - ** leak. `base' should not be allocated in the first place if `zone' is
> - ** to be reused. See also the workaround in MR_unget_zone().
> - */
> zone->MR_zone_bottom = base;
>
> -#ifdef MR_PROTECTPAGE
> - total_size = size + MR_unit;
> -#else
> - total_size = size;
> -#endif /* MR_PROTECTPAGE */
> -
> zone->MR_zone_top = (MR_Word *) ((char *) base + total_size);
> zone->MR_zone_min = (MR_Word *) ((char *) base + offset);
> #ifdef MR_LOWLEVEL_DEBUG
> @@ -593,6 +795,9 @@ MR_extend_zone(MR_MemoryZone *zone, size
> MR_fatal_error("MR_extend_zone called with NULL pointer");
> }
>
> + /*
> + ** Why use this value for new_total_size?
> + */
XXX?
> #ifdef MR_PROTECTPAGE
> new_total_size = new_size + 2 * MR_unit;
> #else
> @@ -645,25 +850,34 @@ MR_extend_zone(MR_MemoryZone *zone, size
> return base_incr;
> }
...
> Index: runtime/mercury_memory_zones.h
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_memory_zones.h,v
> retrieving revision 1.20
> diff -u -p -b -r1.20 mercury_memory_zones.h
> --- runtime/mercury_memory_zones.h 5 Sep 2008 11:19:33 -0000 1.20
> +++ runtime/mercury_memory_zones.h 26 May 2010 07:54:44 -0000
> @@ -24,6 +24,7 @@
>
> #include "mercury_types.h" /* for MR_Word */
> #include "mercury_std.h" /* for MR_bool */
> +#include "mercury_atomic_ops.h" /* for MR_THREADSAFE_VOLATILE */
>
> typedef struct MR_MemoryZone_Struct MR_MemoryZone;
>
> @@ -98,9 +99,9 @@ typedef MR_bool MR_ZoneHandler(MR_Word *
> */
>
> struct MR_MemoryZone_Struct {
> - MR_MemoryZone *MR_zone_next;
> + MR_MemoryZone * MR_THREADSAFE_VOLATILE MR_zone_next;
> const char *MR_zone_name;
> - int MR_zone_id;
> + MR_Unsigned MR_zone_id;
> size_t MR_zone_desired_size;
> size_t MR_zone_redzone_size;
> MR_Word *MR_zone_bottom;
> @@ -134,6 +135,22 @@ struct MR_MemoryZones_Struct {
> MR_MemoryZones *MR_zones_tail;
> };
>
> +/*
> +** Free memory zones are arranged in a list of lists, The outer list (below)
> +** associates a size of the zones in each inner list. It is to be kept in
> +** sorted order from smallest to largest. So that a traversal of this list
> +** returns the zones that are the 'best fit' as the 'first fit'. The inner
> +** lists (using the MR_zone_next field of the zones contain zones of all the
> +** same size.
> +*/
Elsewhere you refer to the outer list as the major list, and the inner
list as the minor list.
> +typedef struct MR_MemoryZonesFree_Struct MR_MemoryZonesFree;
> +
> +struct MR_MemoryZonesFree_Struct {
> + size_t MR_zonesfree_size;
> + MR_MemoryZone *MR_zonesfree_head;
> + MR_MemoryZonesFree *MR_zonesfree_tail;
> +};
> +
> /*
> ** MR_zone_end specifies the end of the area accessible without
> ** a page fault. It is used by MR_clear_zone_for_GC().
> @@ -198,28 +215,16 @@ extern void MR_init_zones(void);
> ** If it fails to allocate or protect the zone, then it exits.
> ** If MR_CHECK_OVERFLOW_VIA_MPROTECT is unavailable, then the last two
> ** arguments are ignored.
> +**
> +** This may re-use previously allocated memory but will re-configure the
> +** name, readzone and handler.
s/readzone/redzone/
...
> Index: runtime/mercury_misc.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_misc.c,v
> retrieving revision 1.28
> diff -u -p -b -r1.28 mercury_misc.c
> --- runtime/mercury_misc.c 14 Nov 2006 00:15:40 -0000 1.28
> +++ runtime/mercury_misc.c 26 May 2010 07:54:44 -0000
> @@ -88,9 +88,13 @@ void
> MR_fatal_error(const char *fmt, ...)
> {
> va_list args;
> + int error = errno;
>
> fflush(stdout); /* in case stdout and stderr are the same */
>
> + if (error != 0) {
> + fprintf(stderr, "Errno: %s\n", strerror(error));
> + }
I suggest also printing errno itself there, sometimes the description
won't be that useful, e.g. when it's something like "Unknown error".
> fprintf(stderr, "Mercury runtime: ");
> va_start(args, fmt);
> vfprintf(stderr, fmt, args);
Please post a revised log message and diff once you have addressed the
above.
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to: mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions: mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------
More information about the reviews
mailing list