NupControl improvements to prevent save/restore problems.

The NupControl string needs to be a reference counted to work with save/
restore, and when the parameter is changed the new value needs to be
propagated to children devices. The structure containing the string is
adpated (simplified slightly) from the PageList implementation.

Note that the parents and children references must be included in the ref_count
when the NupControl structure is updated (example if 'epo' is installed, when
it uninstalls itself, cannot free the the structure when it does rc_decrement).

Minor comment change to devices/gdevbit.c to change NupList to NupControl.

9 files changed, 138 insertions, 41 deletions

diff --git a/base/gdevdflt.c b/base/gdevdflt.c
index 310b716c5..5267b7589 100644
--- a/base/gdevdflt.c
+++ b/base/gdevdflt.c
@@ -1431,6 +1431,8 @@ int gx_device_subclass(gx_device *dev_to_subclass, gx_device *new_prototype, uns
         rc_increment(dev_to_subclass->icc_struct);
     if (dev_to_subclass->PageList)
         rc_increment(dev_to_subclass->PageList);
+    if (dev_to_subclass->NupControl)
+        rc_increment(dev_to_subclass->NupControl);
 
     /* In case the new device we're creating has already been initialised, copy
      * its additional data.
@@ -1561,6 +1563,8 @@ int gx_device_unsubclass(gx_device *dev)
             rc_decrement(child->icc_struct, "gx_unsubclass_device, icc_struct");
         if (child->PageList)
             rc_decrement(child->PageList, "gx_unsubclass_device, PageList");
+        if (child->NupControl)
+            rc_decrement(child->NupControl, "gx_unsubclass_device, NupControl");
         /* we cannot afford to free the child device if its stype is not dynamic because
          * we can't 'null' the finalise routine, and we cannot permit the device to be finalised
          * because we have copied it up one level, not discarded it.
diff --git a/base/gdevkrnlsclass.c b/base/gdevkrnlsclass.c
index 60403f53e..e6f0c79c7 100644
--- a/base/gdevkrnlsclass.c
+++ b/base/gdevkrnlsclass.c
@@ -57,7 +57,7 @@ int install_internal_subclass_devices(gx_device **ppdev, int *devices_loaded)
         while(dev->parent)
             dev = dev->parent;
 
-        /* Note in all devices in chain that we have loaded the PageHandler */
+        /* Note in all devices in chain that we have loaded the NupHandler */
         do {
             dev->NupHandlerPushed = true;
             dev = dev->child;
diff --git a/base/gdevnup.c b/base/gdevnup.c
index 970a9e360..ac569c89f 100644
--- a/base/gdevnup.c
+++ b/base/gdevnup.c
@@ -54,6 +54,11 @@ RELOC_PTRS_END
 
 public_st_nup_device();
 
+
+/**************************************************************************************/
+/* Externals not in headers                                                           */
+/* Imported from gsdparam.c                                                           */
+extern void rc_free_NupControl(gs_memory_t * mem, void *ptr_in, client_name_t cname);
 /**************************************************************************************/
 
 /* This device is one of the 'subclassing' devices, part of a chain or pipeline
@@ -199,8 +204,8 @@ ParseNupControl(gx_device *dev, Nup_device_subclass_data *pNup_data)
         return 0;
     }
     /* First parse the NupControl string for our parameters */
-    if (sscanf(dev->NupControl, "%dx%d", &(pNup_data->NupH), &(pNup_data->NupV)) != 2) {
-        emprintf1(dev->memory, "*** Invalid NupControl format '%s'\n", dev->NupControl);
+    if (sscanf(dev->NupControl->nupcontrol_str, "%dx%d", &(pNup_data->NupH), &(pNup_data->NupV)) != 2) {
+        emprintf1(dev->memory, "*** Invalid NupControl format '%s'\n", dev->NupControl->nupcontrol_str);
         nup_disable_nesting(pNup_data);
         return_error(gs_error_unknownerror);
     }
@@ -386,6 +391,7 @@ nup_put_params(gx_device *dev, gs_param_list * plist)
     const float *res = dev->HWResolution;
     gs_param_string nuplist;
     Nup_device_subclass_data* pNup_data = dev->subclass_data;
+    gx_device *next_dev;
 
 #if 0000
 gs_param_list_dump(plist);
@@ -396,28 +402,49 @@ gs_param_list_dump(plist);
         ecode = code;
 
     if (code == 0) {
-        if (dev->NupControl && (
-            nuplist.size == 0 ||
-            (strncmp(dev->NupControl, (const char *)nuplist.data, nuplist.size) != 0))) {
-            /* There was a NupControl, but this one is different -- free the old one */
-            gs_free(dev->memory->non_gc_memory, dev->NupControl, 1,
-                    strlen(dev->NupControl) + 1, "previous NupControl string");
-
+        if (dev->NupControl && (nuplist.size == 0 ||
+            (strncmp(dev->NupControl->nupcontrol_str, (const char *)nuplist.data, nuplist.size) != 0))) {
             /* If we have accumulated a nest when the NupControl changes, flush the nest */
             if (pNup_data->PagesPerNest > 1 && pNup_data->PageCount > 0)
                 code = nup_flush_nest_to_output(dev, pNup_data, true);
             if (code < 0)
                 ecode = code;
+            /* There was a NupControl, but this one is different -- no longer use the old one */
+            rc_decrement(dev->NupControl, "default put_params NupControl");
             dev->NupControl = 0;
         }
         if (dev->NupControl == NULL && nuplist.size > 0) {
-            /* Allocate a string (with room for terminating NUL) in non_gc_memory */
-            dev->NupControl = (char *)gs_alloc_bytes(dev->memory->non_gc_memory,
-                                                     nuplist.size + 1, "NupControl string");
-            if (!dev->NupControl)
+            dev->NupControl = (gdev_nupcontrol *)gs_alloc_bytes(dev->memory->non_gc_memory,
+                                                              sizeof(gdev_nupcontrol), "structure to hold nupcontrol_str");
+            if (dev->NupControl == NULL)
+                return gs_note_error(gs_error_VMerror);
+            dev->NupControl->nupcontrol_str = (void *)gs_alloc_bytes(dev->memory->non_gc_memory,
+                                                                     nuplist.size + 1, "nupcontrol string");
+            if (dev->NupControl->nupcontrol_str == NULL){
+                gs_free(dev->memory->non_gc_memory, dev->NupControl, 1, sizeof(gdev_nupcontrol),
+                        "free structure to hold nupcontrol string");
+                dev->NupControl = 0;
                 return gs_note_error(gs_error_VMerror);
-            memset(dev->NupControl, 0x00, nuplist.size + 1);
-            memcpy(dev->NupControl, nuplist.data, nuplist.size);
+            }
+            memset(dev->NupControl->nupcontrol_str, 0x00, nuplist.size + 1);
+            memcpy(dev->NupControl->nupcontrol_str, nuplist.data, nuplist.size);
+            rc_init_free(dev->NupControl, dev->memory->non_gc_memory, 1, rc_free_NupControl);
+        }
+        /* Propagate the new NupControl struct to children so get_params has a valid param */
+        next_dev = dev->child;
+        while (next_dev != NULL) {
+            rc_decrement(next_dev->NupControl, "nup_put_params");
+            next_dev->NupControl = dev->NupControl;
+            rc_increment(next_dev->NupControl);
+            next_dev = next_dev->child;
+        }
+        /* Propagate the new NupControl struct to parents so get_params has a valid param */
+        next_dev = dev->parent;
+        while (next_dev != NULL) {
+            rc_decrement(next_dev->NupControl, "nup_put_params");
+            next_dev->NupControl = dev->NupControl;
+            rc_increment(next_dev->NupControl);
+            next_dev = next_dev->parent;
         }
         if (ecode < 0)
             return ecode;
diff --git a/base/gdevsclass.c b/base/gdevsclass.c
index f5430e292..b5b393f2e 100644
--- a/base/gdevsclass.c
+++ b/base/gdevsclass.c
@@ -888,4 +888,6 @@ void default_subclass_finalize(const gs_memory_t *cmem, void *vptr)
         rc_decrement(dev->icc_struct, "finalize subclass device");
     if (dev->PageList)
         rc_decrement(dev->PageList, "finalize subclass device");
+    if (dev->NupControl)
+        rc_decrement(dev->NupControl, "finalize subclass device");
 }
diff --git a/base/gsdevice.c b/base/gsdevice.c
index d3f54e70d..5796db475 100644
--- a/base/gsdevice.c
+++ b/base/gsdevice.c
@@ -71,6 +71,10 @@ gx_device_finalize(const gs_memory_t *cmem, void *vptr)
         rc_decrement(dev->PageList, "gx_device_finalize(PageList)");
         dev->PageList = 0;
     }
+    if (dev->NupControl) {
+        rc_decrement(dev->NupControl, "gx_device_finalize(NupControl)");
+        dev->NupControl = 0;
+    }
 
     if (dev->finalize)
         dev->finalize(dev);
diff --git a/base/gsdparam.c b/base/gsdparam.c
index 656c19cb6..c3b784263 100644
--- a/base/gsdparam.c
+++ b/base/gsdparam.c
@@ -25,6 +25,8 @@
 #include "gxdevsop.h"
 #include "gxfixed.h"
 #include "gsicc_manage.h"
+#include "gdevnup.h"		/* to install N-up subclass device */
+extern gx_device_nup gs_nup_device;
 
 /* Define whether we accept PageSize as a synonym for MediaSize. */
 /* This is for backward compatibility only. */
@@ -483,12 +485,15 @@ int gx_default_get_param(gx_device *dev, char *Param, void *list)
         return param_write_bool(plist, "DisablePageHandler", &temp_bool);
     }
     if (strcmp(Param, "NupControl") == 0){
-        gs_param_string nuplist;
-        if (dev->NupControl)
-            param_string_from_string(nuplist, dev->NupControl);
+        gs_param_string nupcontrol;
+
+        if (dev->NupControl) {
+            gdev_nupcontrol *p = (gdev_nupcontrol *)dev->NupControl;
+            param_string_from_string(nupcontrol, p->nupcontrol_str);
+        }
         else
-            param_string_from_string(nuplist, null_str);
-        return param_write_string(plist,"NupControl", &nuplist);
+            param_string_from_string(nupcontrol, null_str);
+        return param_write_string(plist, "NupControl", &nupcontrol);
     }
     if (strcmp(Param, "PageList") == 0){
         gs_param_string pagelist;
@@ -786,19 +791,21 @@ gx_default_get_params(gx_device * dev, gs_param_list * plist)
     if ((code = param_write_bool(plist, "DisablePageHandler", &temp_bool)) < 0)
         return code;
 
-    if (dev->NupControl)
-        param_string_from_string(nuplist, dev->NupControl);
-    else
+    if (dev->NupControl) {
+        gdev_nupcontrol *p = (gdev_nupcontrol *)dev->NupControl;
+        param_string_from_string(nuplist, p->nupcontrol_str);
+    } else {
         param_string_from_string(nuplist, null_str);
+    }
     if ((code = param_write_string(plist, "NupControl", &nuplist)) < 0)
         return code;
 
     if (dev->PageList) {
         gdev_pagelist *p = (gdev_pagelist *)dev->PageList;
         param_string_from_string(pagelist, p->Pages);
-    }
-    else
+    } else {
         param_string_from_string(pagelist, null_str);
+    }
     if ((code = param_write_string(plist, "PageList", &pagelist)) < 0)
         return code;
 
@@ -1437,6 +1444,19 @@ gx_default_put_icc(gs_param_string *icc_pro, gx_device * dev,
     return code;
 }
 
+void rc_free_NupControl(gs_memory_t * mem, void *ptr_in, client_name_t cname);	/* silence warning */
+/* Exported for use by nup_put_params in gdevnup.c */
+void
+rc_free_NupControl(gs_memory_t * mem, void *ptr_in, client_name_t cname)
+{
+    gdev_nupcontrol *pnupc = (gdev_nupcontrol *)ptr_in;
+
+    if (pnupc->rc.ref_count <= 1) {
+        gs_free(mem->non_gc_memory, pnupc->nupcontrol_str, 1, strlen(pnupc->nupcontrol_str), "free nupcontrol string");
+        gs_free(mem->non_gc_memory, pnupc, 1, sizeof(gdev_nupcontrol), "free structure to hold nupcontrol string");
+    }
+}
+
 static void
 rc_free_pages_list(gs_memory_t * mem, void *ptr_in, client_name_t cname)
 {
@@ -2012,25 +2032,58 @@ label:\
     if (code == 0)
         dev->DisablePageHandler = temp_bool;
 
+    /* If we have an NupControl subclass device (N-up) installed, this param will have	*/
+    /* been handled there, so the check for different will be false, meaning that this	*/
+    /* code won't end up doing anything. This will catch the first occurence and needs	*/
+    /* to install the N-up subclass device.						*/
     code = param_read_string(plist, "NupControl", &nuplist);
     if (code < 0)
         ecode = code;
-
     if (code == 0) {
-        if (dev->NupControl && (strncmp(dev->NupControl, (const char *)nuplist.data, nuplist.size) != 0)) {
-            /* There was a NupControl, but this one is different -- free the old one */
-            gs_free(dev->memory->non_gc_memory, dev->NupControl, 1,
-                    strlen(dev->NupControl) + 1, "previous NupControl string");
+        if (dev->NupControl && (
+            nuplist.size == 0 ||
+            (strncmp(dev->NupControl->nupcontrol_str, (const char *)nuplist.data, nuplist.size) != 0))) {
+            /* There was a NupControl, but this one is different -- no longer use the old one */
+            rc_decrement(dev->NupControl, "default put_params NupControl");
+            dev->NupControl = NULL;
+        }
+    }
+    if (dev->NupControl == NULL && code == 0 && nuplist.size > 0) {
+        gx_device *next_dev;
+
+        dev->NupControl = (gdev_nupcontrol *)gs_alloc_bytes(dev->memory->non_gc_memory,
+                                                          sizeof(gdev_nupcontrol), "structure to hold nupcontrol_str");
+        if (dev->NupControl == NULL)
+            return gs_note_error(gs_error_VMerror);
+        dev->NupControl->nupcontrol_str = (void *)gs_alloc_bytes(dev->memory->non_gc_memory,
+                                                                 nuplist.size + 1, "nupcontrol string");
+        if (dev->NupControl->nupcontrol_str == NULL){
+            gs_free(dev->memory->non_gc_memory, dev->NupControl, 1, sizeof(gdev_nupcontrol),
+                    "free structure to hold nupcontrol string");
             dev->NupControl = 0;
+            return gs_note_error(gs_error_VMerror);
+        }
+        memset(dev->NupControl->nupcontrol_str, 0x00, nuplist.size + 1);
+        memcpy(dev->NupControl->nupcontrol_str, nuplist.data, nuplist.size);
+        rc_init_free(dev->NupControl, dev->memory->non_gc_memory, 1, rc_free_NupControl);
+
+        /* Propagate the new NupControl struct to children */
+        next_dev = dev->child;
+        while (next_dev != NULL) {
+            if (next_dev->NupControl)
+                rc_decrement(next_dev->NupControl, "nup_put_params");
+            next_dev->NupControl = dev->NupControl;
+            rc_increment(dev->NupControl);
+            next_dev = next_dev->child;
         }
-        if (dev->NupControl == NULL && nuplist.size > 0) {
-            /* Allocate a string (with room for terminating NUL) in non_gc_memory */
-            dev->NupControl = (char *)gs_alloc_bytes(dev->memory->non_gc_memory,
-                                                     nuplist.size + 1, "NupControl string");
-            if (!dev->NupControl)
-                return gs_note_error(gs_error_VMerror);
-            memset(dev->NupControl, 0x00, nuplist.size + 1);
-            memcpy(dev->NupControl, nuplist.data, nuplist.size);
+        /* Propagate the new NupControl struct to parents */
+        next_dev = dev->parent;
+        while (next_dev != NULL) {
+            if (next_dev->NupControl)
+                rc_decrement(next_dev->NupControl, "nup_put_params");
+            next_dev->NupControl = dev->NupControl;
+            rc_increment(dev->NupControl);
+            next_dev = next_dev->parent;
         }
     }
 
diff --git a/base/gslibctx.c b/base/gslibctx.c
index c12e624c2..d72826122 100644
--- a/base/gslibctx.c
+++ b/base/gslibctx.c
@@ -1181,6 +1181,8 @@ gs_lib_ctx_stash_sanitized_arg(gs_lib_ctx_t *ctx, const char *arg)
                 break;
             if (ARG_MATCHES("ColorConversionStrategy", arg+2, p-arg-3))
                 break;
+            if (ARG_MATCHES("NupControl", arg+2, p-arg-3))
+                break;
             if (ARG_MATCHES("PageList", arg+2, p-arg-3))
                 break;
             if (ARG_MATCHES("ProcessColorModel", arg+2, p-arg-3))
diff --git a/base/gxdevcli.h b/base/gxdevcli.h
index 91c9ae887..fe7095574 100644
--- a/base/gxdevcli.h
+++ b/base/gxdevcli.h
@@ -706,6 +706,11 @@ typedef struct gdev_space_params_s {
 int gdev_space_params_cmp(const gdev_space_params sp1,
                           const gdev_space_params sp2);
 
+typedef struct gdev_nupcontrol_s {
+        rc_header rc;
+        char *nupcontrol_str;		/* NUL termintated string */
+} gdev_nupcontrol;
+
 typedef struct gdev_pagelist_s {
         rc_header rc;
         char *Pages;
@@ -759,7 +764,7 @@ typedef struct gdev_pagelist_s {
         bool DisablePageHandler;   /* Can be set by the interpreter if it will process FirstPage and LastPage itself */\
         int ObjectFilter;          /* Bit field for which object filters to apply */\
         bool ObjectHandlerPushed;  /* Handles filtering of objects to devices */\
-        char *NupControl;          /* NUL terminated string parameter for Nup */\
+        gdev_nupcontrol *NupControl;\
         bool NupHandlerPushed;     /* Handles Nup operations */\
         long PageCount;			/* number of pages written */\
         long ShowpageCount;		/* number of calls on showpage */\
diff --git a/base/gxdevice.h b/base/gxdevice.h
index b833ed9d3..1b09334b8 100644
--- a/base/gxdevice.h
+++ b/base/gxdevice.h
@@ -121,7 +121,7 @@
 #define std_device_part3_()\
         0/*FirstPage*/, 0/*LastPage*/, 0/*PageHandlerPushed*/, 0/*DisablePageHandler*/,\
         0/* Object Filter*/, 0/*ObjectHandlerPushed*/,\
-        0/* NupControl */, 0/* NupHandlerPushed */,\
+        0, /* NupControl */ 0, /* NupHandlerPushed */\
         0/*PageCount*/, 0/*ShowpageCount*/, 1/*NumCopies*/, 0/*NumCopies_set*/,\
         0/*IgnoreNumCopies*/, 0/*UseCIEColor*/, 0/*LockSafetyParams*/,\
         0/*band_offset_x*/, 0/*band_offset_y*/, false /*BLS_force_memory*/, \