pnstrdup considered armed and dangerous

On Mon, Oct 3, 2016 at 5:55 PM, Andres Freund <andres@anarazel.de> wrote:

/*
* pnstrdup
* Like pstrdup(), but append null byte to a
* not-necessarily-null-terminated input string.
*/
char *
pnstrdup(const char *in, Size len)
{
char *out = palloc(len + 1);

memcpy(out, in, len);
out[len] = '\0';
return out;
}

isn't that a somewhat weird behaviour / implementation? Not really like
strndup(), which one might believe to be analoguous...

Yikes!

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 3 October 2016 at 22:55, Andres Freund <andres@anarazel.de> wrote:

A colleage of me just wrote innocent looking code like
char *shardRelationName = pnstrdup(relationName, NAMEDATALEN);
which is at the moment wrong if relationName isn't preallocated to
NAMEDATALEN size.

[snip]

isn't that a somewhat weird behaviour / implementation? Not really like
strndup(), which one might believe to be analoguous...

Well I wouldn't say it's wrong, exactly. It might produce a segfault
if relationName[NAMEDATALEN] is outside readable memory for the
process, but otherwise it will behave as defined.

Geoff

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 4 October 2016 at 14:12, Geoff Winkless <pgsqladmin@geoff.dj> wrote:

Well I wouldn't say it's wrong, exactly. It might produce a segfault
if relationName[NAMEDATALEN] is outside readable memory for the
process, but otherwise it will behave as defined.

Finger slippage. Of course I meant

... if relationName[NAMEDATALEN-1] is outside...

Geoff

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2016-10-03 14:55:24 -0700, Andres Freund wrote:

Hi,

A colleage of me just wrote innocent looking code like
char *shardRelationName = pnstrdup(relationName, NAMEDATALEN);
which is at the moment wrong if relationName isn't preallocated to
NAMEDATALEN size.

/*
* pnstrdup
* Like pstrdup(), but append null byte to a
* not-necessarily-null-terminated input string.
*/
char *
pnstrdup(const char *in, Size len)
{
char *out = palloc(len + 1);

memcpy(out, in, len);
out[len] = '\0';
return out;
}

isn't that a somewhat weird behaviour / implementation? Not really like
strndup(), which one might believe to be analoguous...

I've since hit this bug again. To fix it, you'd need strnlen. The lack
of which I'd also independently hit twice. So here's a patch adding
pg_strnlen and using that to fix pnstrdup.

Greetings,

Andres Freund