trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zellerh <...@git.apache.org>
Subject [GitHub] incubator-trafodion pull request #833: jira 2227 : support json fuction - JS...
Date Mon, 14 Nov 2016 21:58:58 GMT
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/833#discussion_r87902217
  
    --- Diff: core/sql/common/stringinfo.cpp ---
    @@ -0,0 +1,258 @@
    +#include "stringinfo.h"
    +#include <stdlib.h>
    +#include <string.h>
    +#include "str.h"
    +
    +/*
    + * makeStringInfo
    + *
    + * Create an empty 'StringInfoData' & return a pointer to it.
    + */
    +StringInfo
    +makeStringInfo(void)
    +{
    +    StringInfo	res;
    +
    +    res = (StringInfo) malloc(sizeof(StringInfoData));
    +
    +    initStringInfo(res);
    +
    +    return res;
    +}
    +
    +/*
    + * initStringInfo
    + *
    + * Initialize a StringInfoData struct (with previously undefined contents)
    + * to describe an empty string.
    + */
    +void
    +initStringInfo(StringInfo str)
    +{
    +    int			size = 1024;	/* initial default buffer size */
    +
    +    str->data = (char *) malloc(size);
    +    str->maxlen = size;
    +    resetStringInfo(str);
    +}
    +
    +/*
    + * resetStringInfo
    + *
    + * Reset the StringInfo: the data buffer remains valid, but its
    + * previous content, if any, is cleared.
    + */
    +void
    +resetStringInfo(StringInfo str)
    +{
    +    str->data[0] = '\0';
    +    str->len = 0;
    +    str->cursor = 0;
    +}
    +
    +/*
    + * appendStringInfo
    + *
    + * Format text data under the control of fmt (an sprintf-style format string)
    + * and append it to whatever is already in str.  More space is allocated
    + * to str if necessary.  This is sort of like a combination of sprintf and
    + * strcat.
    + */
    +void
    +appendStringInfo(StringInfo str, const char *fmt, ...)
    +{
    +    for (;;)
    +    {
    +        va_list		args;
    +        int			needed;
    +
    +        /* Try to format the data. */
    +        va_start(args, fmt);
    +        needed = appendStringInfoVA(str, fmt, args);
    +        va_end(args);
    +
    +        if (needed == 0)
    +            break;				/* success */
    +
    +        /* Increase the buffer size and try again. */
    +        enlargeStringInfo(str, needed);
    +    }
    +}
    +
    +/*
    + * appendStringInfoVA
    + *
    + * Attempt to format text data under the control of fmt (an sprintf-style
    + * format string) and append it to whatever is already in str.  If successful
    + * return zero; if not (because there's not enough space), return an estimate
    + * of the space needed, without modifying str.  Typically the caller should
    + * pass the return value to enlargeStringInfo() before trying again; see
    + * appendStringInfo for standard usage pattern.
    + *
    + * XXX This API is ugly, but there seems no alternative given the C spec's
    + * restrictions on what can portably be done with va_list arguments: you have
    + * to redo va_start before you can rescan the argument list, and we can't do
    + * that from here.
    + */
    +int
    +appendStringInfoVA(StringInfo str, const char *fmt, va_list args)
    +{
    +    int			avail;
    +    size_t		nprinted;
    +
    +    if (str == NULL)
    +        return 0;
    +
    +    /*
    +     * If there's hardly any space, don't bother trying, just fail to make the
    +     * caller enlarge the buffer first.  We have to guess at how much to
    +     * enlarge, since we're skipping the formatting work.
    +     */
    +    avail = str->maxlen - str->len;
    +    if (avail < 16)
    +        return 32;
    +
    +    //nprinted = pvsnprintf(str->data + str->len, (size_t) avail, fmt, args);
    +    nprinted = snprintf(str->data + str->len, (size_t) avail, fmt, args);
    +
    +    if (nprinted < (size_t) avail)
    +    {
    +        /* Success.  Note nprinted does not include trailing null. */
    +        str->len += (int) nprinted;
    +        return 0;
    +    }
    +
    +    /* Restore the trailing null so that str is unmodified. */
    +    str->data[str->len] = '\0';
    +
    +    /*
    +     * Return pvsnprintf's estimate of the space needed.  (Although this is
    +     * given as a size_t, we know it will fit in int because it's not more
    +     * than MaxAllocSize.)
    +     */
    +    return (int) nprinted;
    +}
    +
    +/*
    + * appendStringInfoChar
    + *
    + * Append a single byte to str.
    + * Like appendStringInfo(str, "%c", ch) but much faster.
    + */
    +void
    +appendStringInfoChar(StringInfo str, char ch)
    +{
    +    /* Make more room if needed */
    +    if (str->len + 1 >= str->maxlen)
    +        enlargeStringInfo(str, 1);
    +
    +    /* OK, append the character */
    +    str->data[str->len] = ch;
    +    str->len++;
    +    str->data[str->len] = '\0';
    +}
    +
    +
    +/*
    + * appendStringInfoString
    + *
    + * Append a null-terminated string to str.
    + * Like appendStringInfo(str, "%s", s) but faster.
    + */
    +void
    +appendStringInfoString(StringInfo str, const char *s)
    +{
    +    appendBinaryStringInfo(str, s, str_len(s));
    +}
    +
    +
    +/*
    + * appendBinaryStringInfo
    + *
    + * Append arbitrary binary data to a StringInfo, allocating more space
    + * if necessary.
    + */
    +void
    +appendBinaryStringInfo(StringInfo str, const char *data, int datalen)
    +{
    +    //ASSERT(str != NULL);
    +
    +    /* Make more room if needed */
    +    enlargeStringInfo(str, datalen);
    +
    +    /* OK, append the data */
    +    memcpy(str->data + str->len, data, datalen);
    +    str->len += datalen;
    +
    +    /*
    +     * Keep a trailing null in place, even though it's probably useless for
    +     * binary data.  (Some callers are dealing with text but call this because
    +     * their input isn't null-terminated.)
    +     */
    +    str->data[str->len] = '\0';
    +}
    +
    +
    +/*
    + * enlargeStringInfo
    + *
    + * Make sure there is enough space for 'needed' more bytes
    + * ('needed' does not include the terminating null).
    + *
    + * External callers usually need not concern themselves with this, since
    + * all stringinfo.c routines do it automatically.  However, if a caller
    + * knows that a StringInfo will eventually become X bytes large, it
    + * can save some palloc overhead by enlarging the buffer before starting
    + * to store data in it.
    + *
    + * NB: because we use repalloc() to enlarge the buffer, the string buffer
    + * will remain allocated in the same memory context that was current when
    + * initStringInfo was called, even if another context is now current.
    + * This is the desired and indeed critical behavior!
    + */
    +void
    +enlargeStringInfo(StringInfo str, int needed)
    +{
    +    int			newlen;
    +
    +    /*
    +     * Guard against out-of-range "needed" values.  Without this, we can get
    +     * an overflow or infinite loop in the following.
    +     */
    +    if (needed < 0)				/* should not happen */
    +        return;
    +    if (((size_t) needed) >= (MaxAllocSize - (size_t) str->len))
    +        return;
    +
    +    needed += str->len + 1;		/* total space required now */
    +
    +    /* Because of the above test, we now have needed <= MaxAllocSize */
    +
    +    if (needed <= str->maxlen)
    +        return;					/* got enough space already */
    +
    +    /*
    +     * We don't want to allocate just a little more space with each append;
    +     * for efficiency, double the buffer size each time it overflows.
    +     * Actually, we might need to more than double it if 'needed' is big...
    +     */
    +    newlen = 2 * str->maxlen;
    +    while (needed > newlen)
    +        newlen = 2 * newlen;
    +
    +    /*
    +     * Clamp to MaxAllocSize in case we went past it.  Note we are assuming
    +     * here that MaxAllocSize <= INT_MAX/2, else the above loop could
    +     * overflow.  We will still have newlen >= needed.
    +     */
    +    if (newlen > (int) MaxAllocSize)
    +        newlen = (int) MaxAllocSize;
    +
    +    if (str->data != NULL)
    +        free(str->data);
    +    str->data = (char *) malloc(newlen);
    --- End diff --
    
    Sorry, I haven't reviewed all of the code. Is there a chance that we will introduce memory
leaks? Would it make sense to pass a Trafodion heap pointer (NAMemory *) to a StringInfo object,
so we can control the memory allocation better?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message