Re: 【ECPG】strncpy function does not set the end character '\0'

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Meskes <meskes(at)postgresql(dot)org>
Cc: "postgresql_2016(at)163(dot)com" <postgresql_2016(at)163(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: 【ECPG】strncpy function does not set the end character '\0'
Date: 2017-09-06 12:14:41
Message-ID: 8615.1504700081@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Michael Meskes <meskes(at)postgresql(dot)org> writes:
>> In the function ECPGnoticeReceiver, we use the stncpy function copy
>> the
>> sqlstate to sqlca->sqlstate. And the sqlca->sqlstate is defined as
>> the size
>> of 5, and the copy size is sizeof(sqlca->sqlstate). However, from the
>> previous strcmp function, the sqlstate size may be 5,such as
>> ECPG_SQLSTATE_INVALID_CURSOR_NAME. So there may be lack of the end
>> character
>> for sqlca->sqlstate.

> Why do you think there should be one? My memory might be wrong but I
> don't think it's supposed to be a null terminated string.

That field is defined as char[5] in struct sqlca_t, so the intent is
clearly that it not be null terminated. However, it looks to me like
there'd be at least one alignment-padding byte after it, and that byte
is likely to be 0 in a lot of situations (particularly for statically
allocated sqlca_t's). So a lot of the time, you could get away with
using strcmp() or other functions that expect null termination.

I'm thinking therefore that there's probably code out there that tries
to do strcmp(sqlca->sqlstate, "22000") or suchlike, and it works often
enough that the authors haven't identified their bug. The question is
do we want to try to make that be valid code.

Changing the field declaration to char[5+1] would be easy enough, but
I have no idea how many places in ecpglib would need to change to make
sure that the last byte gets set to 0.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeevan Ladhe 2017-09-06 12:20:10 Re: Adding support for Default partition in partitioning
Previous Message Jeevan Ladhe 2017-09-06 11:55:18 Re: Adding support for Default partition in partitioning