Is this C style function well written and thread safe?

J

jeff_j_dunlap

Dear C++ Users:

I alwasy use std::string and avoid char buffers but last night I
wanted to see if I could make a C style function that would be thread
safe.

For me, it was a good learning experience since my C/C++ knowledge is
limited but I do understand threading issues due to prior Delphi
experience.

In the following function, pleas assume that the Date object is well
written. What I really want to know is if my char buff is being
handled safely.


void fbDateToStr( const IBPP::Date &d, char *buff )
{
if ( d < IBPP::MinDate || d > IBPP::MaxDate )
{
strcpy(buff, "");
}
else
{
int iMonth=0, iDay=0, iYear=0;
d.GetDate(iYear, iMonth, iDay);
sprintf(buff, "%d/%d/%d", iMonth, iDay, iYear);
}
}


USAGE:
char buffer[15];
fbDateToStr(dtInitialContactDt, buffer);

RETURNS:
MM/DD/YYYY or if date is invalid, a blank string

NOTES:
Initially, I thought of creating a static buffer within the function
instead of passing a buffer as this function currently is doing, but
doing so would have been thread-unsafe since the buffer would now be
visible/editable by all threads.
 
J

jeff_j_dunlap

Dear C++ Users:

I alwasy use std::string and avoid char buffers but last night I
wanted to see if I could make a C style function that would be thread
safe.

For me, it was a good learning experience since my C/C++ knowledge is
limited but I do understand threading issues due to prior Delphi
experience.

In the following function, pleas assume that the Date object is well
written. What I really want to know is if my char buff is being
handled safely.

void fbDateToStr( const IBPP::Date &d, char *buff )
{
if ( d < IBPP::MinDate || d > IBPP::MaxDate )
{
strcpy(buff, "");
}
else
{
int iMonth=0, iDay=0, iYear=0;
d.GetDate(iYear, iMonth, iDay);
sprintf(buff, "%d/%d/%d", iMonth, iDay, iYear);
}

}

USAGE:
char buffer[15];
fbDateToStr(dtInitialContactDt, buffer);

RETURNS:
MM/DD/YYYY or if date is invalid, a blank string

NOTES:
Initially, I thought of creating a static buffer within the function
instead of passing a buffer as this function currently is doing, but
doing so would have been thread-unsafe since the buffer would now be
visible/editable by all threads.

Oh, and another thing that I don't really understand is if I can make
my buffer [10+1] to exactly accomodate the MM/DD/YYYY string size of
10 + 1 for the terminating character?

Thanks again
 
I

Ivan Vecerina

: Dear C++ Users:
:
: I alwasy use std::string and avoid char buffers but last night I
: wanted to see if I could make a C style function that would be thread
: safe.
:
: For me, it was a good learning experience since my C/C++ knowledge is
: limited but I do understand threading issues due to prior Delphi
: experience.
:
: In the following function, pleas assume that the Date object is well
: written. What I really want to know is if my char buff is being
: handled safely.
:
:
: void fbDateToStr( const IBPP::Date &d, char *buff )
: {
: if ( d < IBPP::MinDate || d > IBPP::MaxDate )
: {
: strcpy(buff, "");
Ok, or you could just write: *buff = '\0';

: }
: else
: {
: int iMonth=0, iDay=0, iYear=0;
: d.GetDate(iYear, iMonth, iDay);
: sprintf(buff, "%d/%d/%d", iMonth, iDay, iYear);
This call could cause an unexpected buffer overfow
if iYear/iMonth/iDay has an out-of-range value
(e.g. if year somehow gets to be 12345678 instead of 2007).
Your platform probably provides a call such as
snprintf or slprintf or sprintf_s, which are
all safer by allowing to restrict output size.
[ you can then also more safely rely on a "tight fit" buffer ]

Also, if you want the output to include leading
zeroes (01/01/1999), you'll want to use the following
format string: "%02d/%02d/%04d"

: }
: }
:
:
: USAGE:
: char buffer[15];
: fbDateToStr(dtInitialContactDt, buffer);
:
: RETURNS:
: MM/DD/YYYY or if date is invalid, a blank string
:
: NOTES:
: Initially, I thought of creating a static buffer within the function
: instead of passing a buffer as this function currently is doing, but
: doing so would have been thread-unsafe since the buffer would now be
: visible/editable by all threads.
Indeed: making sure that a buffer of the right size is provided
is a key issue when using C-style character buffers.

Some ideas for dealing with fixed-size char buffers in C++:

You can take a fixed size char array by reference:
void fbDateToStr( const IBPP::Date &d, char (&buff)[11] )
This will check that the caller provides an array of the
exact desired size.

You could also return a character array encapsulated
into a struct. For instance, using something like
boost::array:
array<char,11> fbDateAsStr( IBPP::Date const& d )
{
array<char,11> ans;
...
return ans;
}


hth-Ivan
 
D

dasjotre

For me, it was a good learning experience since my C/C++ knowledge is
limited but I do understand threading issues due to prior Delphi
experience.
In the following function, pleas assume that the Date object is well
written. What I really want to know is if my char buff is being
handled safely.
void fbDateToStr( const IBPP::Date &d, char *buff )
{
if ( d < IBPP::MinDate || d > IBPP::MaxDate )
{
strcpy(buff, "");
}
else
{
int iMonth=0, iDay=0, iYear=0;
d.GetDate(iYear, iMonth, iDay);
sprintf(buff, "%d/%d/%d", iMonth, iDay, iYear);
}

USAGE:
char buffer[15];
fbDateToStr(dtInitialContactDt, buffer);

If buffer is stack allocated than it is thread local
and can not be accessed by other threads, so there
are no concurrency issues.

What do you mean that Date is well written?

what if the thread enteres else block and is
then preempted before GetDate is called,
and another thread then changes d so that

d < IBPP::MinDate || d > IBPP::MaxDate

what values will GetDate initialize iMonth, etc...?

passing d by const reference only means that
you can not call non const members of Date
within fbDateToStr. It doesn't mean that it can
not be changed by some other thread.
RETURNS:
MM/DD/YYYY or if date is invalid, a blank string
NOTES:
Initially, I thought of creating a static buffer within the function
instead of passing a buffer as this function currently is doing, but
doing so would have been thread-unsafe since the buffer would now be
visible/editable by all threads.

Oh, and another thing that I don't really understand is if I can make
my buffer [10+1] to exactly accomodate the MM/DD/YYYY string size of
10 + 1 for the terminating character?

If you are sure that month and day can not be greater than 99
and that the year can not be greater than 9999 then you need
at most 11 character buffer.

DS
 
?

=?ISO-8859-1?Q?Erik_Wikstr=F6m?=

Dear C++ Users:

I alwasy use std::string and avoid char buffers but last night I
wanted to see if I could make a C style function that would be thread
safe.

The easiest way to make things thread safe is to avoid shared data, for
functions this usually means static variables, or parameters passed as
pointers.
For me, it was a good learning experience since my C/C++ knowledge is
limited but I do understand threading issues due to prior Delphi
experience.

In the following function, pleas assume that the Date object is well
written. What I really want to know is if my char buff is being
handled safely.

It is safe if you trust the user, which more or less means that it is
unsafe. The problem with a pointer to a buffer is that it contains no
information about the size of the buffer, which means that you have to
trust the user to supply a large enough buffer. To solve this versions
like snprintf() and strncpy() were created, but once again you have to
trust the user, this time to not lie about the buffer size.
 
A

Andre Kostur

The easiest way to make things thread safe is to avoid shared data, for
functions this usually means static variables, or parameters passed as
pointers.

Uh... static variables are bad for thread safety. Static says there's only
one instance of the variable regardless of how many "instances" of the
function that are currently executing. You want non-static local variables
so that each "instance" of the function has it's own instance of the local
variable.

(Oh yeah... and threading issues are off-topic for comp.lang.c++ ....)
 
J

jeff_j_dunlap

Uh... static variables are bad for thread safety. Static says there's only
one instance of the variable regardless of how many "instances" of the
function that are currently executing. You want non-static local variables
so that each "instance" of the function has it's own instance of the local
variable.

I took it as Erik suggesting to avoid shared data such, i.e. static
vars... but I definately enjoyed the additional information you
provided as to how they are bad for thread saftey.
(Oh yeah... and threading issues are off-topic for comp.lang.c++ ....)

Understood. Although I learned a whole lot from this thread :)

Thanks
 
D

Default User

Andre said:
data, >> > for functions this usually means static variables, or
parameters >> > passed as pointers.

Hmm, I managed to read it as examples of how to avoid shared data,
not examples of shared data.

Ah. Yes, I can see the ambiguity.




Brian
 
O

Old Wolf

: In the following function, pleas assume that the Date object is well
: written. What I really want to know is if my char buff is being
: handled safely.

: int iMonth=0, iDay=0, iYear=0;
: d.GetDate(iYear, iMonth, iDay);
: sprintf(buff, "%d/%d/%d", iMonth, iDay, iYear);

As you say, this is dreadful code because it will
buffer overflow if unexpected values comes from
GetDate. The buff should be at least 3 * INT_BYTES + 3
in length (where INT_BYTES is the number of characters
needed to print INT_MIN). snprintf is a good solution.
Some ideas for dealing with fixed-size char buffers in C++:

You can take a fixed size char array by reference:
void fbDateToStr( const IBPP::Date &d, char (&buff)[11] )
This will check that the caller provides an array of the
exact desired size.

You can achieve the same thing in C with: char (*buff)[11]
 
P

Pete Becker

As you say, this is dreadful code because it will
buffer overflow if unexpected values comes from
GetDate.

But the explicit assumption is that GetDate is "well written," which
certainly implies that it doesn't produce unexpected values. If it
does, the problem is in GetDate, not in the code that assumes that it
does what it's supposed to do. If you don't trust GetDate to meet its
contract, what do you trust it to do?
 
O

Old Wolf

But the explicit assumption is that GetDate is "well written," which
certainly implies that it doesn't produce unexpected values. If it
does, the problem is in GetDate, not in the code that assumes that it
does what it's supposed to do. If you don't trust GetDate to meet its
contract, what do you trust it to do?

As little as possible !

What happens when you link against an upgraded version
of the library that has a bug or behaves slightly differently?

IMHO, it is better to make sure that your own code cannot
cause a buffer overflow, even when poked with a large stick.
 
W

werasm

Andre said:
Uh... static variables are bad for thread safety.

I think that is exactly what he meant - to not have static
members or not pass pointers as parameters, for
static members could be touched by various threads
executing the same function simultaneously, and if
you have pointer parameters, it all depends on what
happens on the outside (is the data shared).
(Oh yeah... and threading issues are off-topic for comp.lang.c++ ....)

It is becoming more and more relevant.
 
J

James Kanze

I alwasy use std::string and avoid char buffers but last night I
wanted to see if I could make a C style function that would be thread
safe.
For me, it was a good learning experience since my C/C++ knowledge is
limited but I do understand threading issues due to prior Delphi
experience.
In the following function, please assume that the Date object is well
written.

It's not. It's missing the most essential part of thread
safety: documentation as to the guarantees it offers:).

In practice, it's what I'd call thread neutral. Threads are
completely irrelevant to it, since it owns and manages no data
of its own. It's up to the user to ensure thread safety when
using it. (For example, if the user passes the address of a
static buffer to buff, it isn't thread safe.) In a
multi-threaded environment, this is what I would consider the
"default guarantee": if a function is documented as being usable
in a multithreaded environment, and provides no other documented
guarantees with regards to threading, this is probably what one
should assume.
What I really want to know is if my char buff is being
handled safely.

What do you mean by "safely"? It's not being handled at all
safely, since you don't guarantee adequate length (and you don't
even tell the user how to ensure adequate length). As far as
threading is concerned, there is no problem if your guarantee is
that the function be thread neutral; it's an out argument, which
you explicitly write, and it is up to the user to ensure that it
isn't accessed from any other thread while you are writing it.

Similar issues concern Date: you only read it, so other threads
should be able to read it simultaneously, but it's up to the
user to ensure that no other thread makes a modifying access
while you are reading it.
void fbDateToStr( const IBPP::Date &d, char *buff )
{
if ( d < IBPP::MinDate || d > IBPP::MaxDate )
{
strcpy(buff, "");
}
else
{
int iMonth=0, iDay=0, iYear=0;
d.GetDate(iYear, iMonth, iDay);
sprintf(buff, "%d/%d/%d", iMonth, iDay, iYear);
}
}
USAGE:
char buffer[15];
fbDateToStr(dtInitialContactDt, buffer);
RETURNS:
MM/DD/YYYY or if date is invalid, a blank string
NOTES:
Initially, I thought of creating a static buffer within the
function instead of passing a buffer as this function
currently is doing, but doing so would have been thread-unsafe
since the buffer would now be visible/editable by all threads.

It's no more thread safe or thread unsafe than what you are
doing. It just presents a different guarantee. Since it isn't
the "default" guarantee, you would have to document the
requirements, but documenting that the user must ensure
synchronization is also a means of achieving thread safety.

Note that thread safety really concerns data, and not functions,
and is relevant to functions only in so far as they might use
data which the user doesn't know about, or in ways which the
user cannot anticipate. When I speak of a function being
"thread neutral", I mean that it doesn't read any data not
passed as an argument, and doesn't write any data not passed as
a documented out argument.
 
P

Pete Becker

As little as possible !

What happens when you link against an upgraded version
of the library that has a bug or behaves slightly differently?

What happens whenever code has a bug? You either hide it by protecting
yourself evey place you use that code, or you fix it in one place. The
lattter is clearly preferable.

If the upgraded version "behaves slightly differently" then its
specification has changed. If you can't rely on its original
specification, you have to review the code that uses it to find out
where you need to make changes.
IMHO, it is better to make sure that your own code cannot
cause a buffer overflow, even when poked with a large stick.

You recommended using snprintf instead of sprintf. How do you protect
yourself against a bug in snprintf?
 
J

James Kanze

On 2007-09-12 18:44:38 -0400, Old Wolf <[email protected]> said:
But the explicit assumption is that GetDate is "well written,"
which certainly implies that it doesn't produce unexpected
values.

For what definition of "unexpected". I don't expect dates 10000
years in the future, but a well written GetDate routine might be
capable of generating them.
If it does, the problem is in GetDate, not in the code that
assumes that it does what it's supposed to do. If you don't
trust GetDate to meet its contract, what do you trust it to
do?

The problem here is that we don't know the contract of GetDate.
A priori, I would expect that the contract would restrict the
possible values for month and day, but would not do so for year;
it makes perfect sense to speak of the year 10000, or even of
the year 100000 (although one might not "expect" such values in
a particular application).

The more general problem is that we've been asked to evaluate
the correctness of a function without being told what the
contract for that function is. Which is an exercise in
futility. About all we can say is that "as it stands", the code
is completely broken, since if I pass it a null pointer, or a
buffer of length 2, bad things will happen, and there is nothing
anywhere which says that such arguments are not allowed.
 
O

Old Wolf

You recommended using snprintf instead of sprintf. How do you protect
yourself against a bug in snprintf?

You assume that standard functions work as documented,
unless you have reason to expect otherwise.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top