op.c - add support for empty RCPV strings.

Currently we have no need for an empty shared string, but there is no
reason it should not be possible. This patch reworks the internals so it
is possible to create one without triggering asserts. Currently we don't
use this, but it seems reasonable that someone might want it in the
future. Under DEBUGGING we will still assert if someone tries to create
an empty RCPV unless the flag specifies it should be allowed.

At the same time the docs for rcpv_new() have been cleaned up a bit to
be more correct and reflect what actually happens inside.

This changes things so that the len member of the RCPV structure is
always non-zero in a well formed structure by accounting for the null
we add to the end explicitly. The RCPV_LEN() macro continues to return
the old value (not including the null).

2 files changed, 50 insertions, 18 deletions

diff --git a/cop.h b/cop.h
index c6af2140c7..618e5724d9 100644
--- a/cop.h
+++ b/cop.h
@@ -534,7 +534,10 @@ string pv created with C<rcpv_new()>.
 Returns the refcount for a pv created with C<rcpv_new()>. 
 
 =for apidoc Am|RCPV *|RCPV_LEN|char *pv
-Returns the length of a pv created with C<rcpv_new()>. 
+Returns the length of a pv created with C<rcpv_new()>.
+Note that this reflects the length of the string from the callers
+point of view, it does not include the mandatory null which is
+always injected at the end of the string by rcpv_new().
 
 =cut
 */
@@ -542,16 +545,19 @@ Returns the length of a pv created with C<rcpv_new()>.
 struct rcpv {
     STRLEN  refcount;  /* UV would mean a 64 refcnt on
                           32 bit builds with -Duse64bitint */
-    STRLEN  len;
+    STRLEN  len;       /* length of string including mandatory
+                          null byte at end */
     char    pv[1];
 };
 typedef struct rcpv RCPV;
 
-#define RCPVf_USE_STRLEN 1
-#define RCPVf_NO_COPY 2
+#define RCPVf_USE_STRLEN    (1 << 0)
+#define RCPVf_NO_COPY       (1 << 1)
+#define RCPVf_ALLOW_EMPTY   (1 << 2)
+
 #define RCPVx(pv_arg)       ((RCPV *)((pv_arg) - STRUCT_OFFSET(struct rcpv, pv)))
 #define RCPV_REFCOUNT(pv)   (RCPVx(pv)->refcount)
-#define RCPV_LEN(pv)        (RCPVx(pv)->len)
+#define RCPV_LEN(pv)        (RCPVx(pv)->len-1) /* len always includes space for a null */
 
 #ifdef USE_ITHREADS
 
diff --git a/op.c b/op.c
index 4b054480e6..eec6da5067 100644
--- a/op.c
+++ b/op.c
@@ -15255,16 +15255,32 @@ Perl_dup_warnings(pTHX_ char* warnings)
 /*
 =for apidoc rcpv_new
 
-Create a new shared memory refcounted string from the argument. If flags is set 
-to RCPVf_USE_STRLEN then the len argument is ignored and set using strlen(pv). 
-If the pv is NULL returns NULL. The newly created string will have a refcount 
-of 1, and is suitable for passing into rcpv_copy() and rcpv_free(). To access
-the RCPV * from the returned value use the RCPVx() macro.
+Create a new shared memory refcounted string with the requested size, and
+with the requested initialization and a refcount of 1. The actual space
+allocated will be 1 byte more than requested and rcpv_new() will ensure that
+the extra byte is a null regardless of any flags settings.
 
-Note that rcpv_new() does NOT use a hash table or anything like that to dedupe
-inputs given the same text content. Each call with a non-null pv parameter
-will produce a distinct pointer with its own refcount regardless of the input
-content.
+If the RCPVf_NO_COPY flag is set then the pv argument will be
+ignored, otherwise the contents of the pv pointer will be copied into
+the new buffer or if it is NULL the function will do nothing and return NULL.
+
+If the RCPVf_USE_STRLEN flag is set then the len argument is ignored and
+recomputed using C<strlen(pv)>. It is an error to combine RCPVf_USE_STRLEN
+and RCPVf_NO_COPY at the same time.
+
+Under DEBUGGING rcpv_new() will assert() if it is asked to create a 0 length
+shared string unless the RCPVf_ALLOW_EMPTY flag is set.
+
+The return value from the function is suitable for passing into rcpv_copy() and
+rcpv_free(). To access the RCPV * from the returned value use the RCPVx() macro.
+The 'len' member of the RCPV struct stores the allocated length (including the
+extra byte), but the RCPV_LEN() macro returns the requested length (not
+including the extra byte).
+
+Note that rcpv_new() does NOT use a hash table or anything like that to
+dedupe inputs given the same text content. Each call with a non-null pv
+parameter will produce a distinct pointer with its own refcount regardless of
+the input content.
 
 =cut
 */
@@ -15277,23 +15293,32 @@ Perl_rcpv_new(pTHX_ const char *pv, STRLEN len, U32 flags) {
 
     PERL_UNUSED_CONTEXT;
 
+    /* Musn't use both at the same time */
+    assert((flags & (RCPVf_NO_COPY|RCPVf_USE_STRLEN))!=
+                    (RCPVf_NO_COPY|RCPVf_USE_STRLEN));
+
     if (!pv && (flags & RCPVf_NO_COPY) == 0)
         return NULL;
 
     if (flags & RCPVf_USE_STRLEN)
         len = strlen(pv);
 
-    rcpv = (RCPV *)PerlMemShared_malloc(sizeof(struct rcpv) + len + 1);
+    assert(len || (flags & RCPVf_ALLOW_EMPTY));
+
+    len++; /* add one for the null we will add to the end */
+
+    rcpv = (RCPV *)PerlMemShared_malloc(sizeof(struct rcpv) + len);
     if (!rcpv)
         croak_no_mem();
 
-    rcpv->len = len;
+    rcpv->len = len;    /* store length including null,
+                           RCPV_LEN() subtracts 1 to account for this */
     rcpv->refcount = 1;
 
     if ((flags & RCPVf_NO_COPY) == 0) {
-        (void)memcpy(rcpv->pv, pv, len);
-        rcpv->pv[len]= '\0';
+        (void)memcpy(rcpv->pv, pv, len-1);
     }
+    rcpv->pv[len-1]= '\0'; /* the last byte should always be null */
     return rcpv->pv;
 }
 
@@ -15324,6 +15349,7 @@ Perl_rcpv_free(pTHX_ char *pv) {
         return NULL;
     RCPV *rcpv = RCPVx(pv);
 
+    assert(rcpv->refcount);
     assert(rcpv->len);
 
     OP_REFCNT_LOCK;