Problem with following function

[Materialised]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the format of:

[Thu May 5 17:06:27 2005]

Here is my function as it stands:

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');
printf("%s\n", curtime);
return curtime;
}

I hope someone can help me.
--
Materialised
perl -e 'printf %silto%c%sal%c%s%ccodegurus%corg%c, ma, 58, mw, 107,
'er', 64, 46, 10;'


Bart: "What's Santa's Little Helper doing to that dog? Looks like he's
trying to jump over, but he can't quite make it."

 

[David Resnick]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the format of:

[Thu May 5 17:06:27 2005]

Here is my function as it stands:

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');
printf("%s\n", curtime);
return curtime;
}

sizeof(curtime+2) is odd usage. It mean the same as sizeof(curtime) I
guess, since the type of curtime+2 is the same as the type of curtime?
Anyway, it yields the number of bytes a pointer to char occupies on
your system, often 4 bytes of 32 bit systems, but not necessarily that.

You want something more like this:
size_t len = strlen(p)+3; /* allow 1 for each char, 1 for '\0' */
curtime = malloc(len);
....
snprintf(curtime, len, "%c%s%c", '[', p, ']');

-David

 

[Thomas Matthews]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the format of:

[Thu May 5 17:06:27 2005]

Here is my function as it stands:

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');

Your problem: sizeof(curtime + 2)
Explanation:
curtime is a pointer.
sizeof(curtime) is the size of the _pointer_.
You want (strlen(p) + 2) instead.

printf("%s\n", curtime);
return curtime;
}

I hope someone can help me.

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.comeaucomputing.com/learn/faq/
Other sites:
http://www.josuttis.com -- C++ STL Library book
http://www.sgi.com/tech/stl -- Standard Template Library

 

[Jens.Toerring]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the format of:

[Thu May 5 17:06:27 2005]

Here is my function as it stands:

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';

First problem: 'e' is an uninitialized pointer, so you can't store
anything where it's currently pointing to. But since 'e' is never
going to be used you can simply remove the whole line.

curtime = malloc(strlen(p)+2);

Since you are going to store a string in 'curtime' you need one
more char for the trailing '\0'. You have to use

curtime = malloc( strlen( p ) + 3 );

if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');

'curtime' is a char pointer, so 'sizeof(curtime)' as well as
'sizeof(curtime+2)' is the amount of memory needed for storing a
char pointer (often 4), not, as you seem to assume, the amount of
memory of what 'curtime' points to (you can't figure that out from
a pointer alone). So make that

snprintf( curtime, strlen( p ) + 3, "%c%s%c", '[', p, ']');

or, perhaps more simple,

snprintf( curtime, strlen( p ) + 3, "[%s]", p');

(use 3 instead of 2 because of the '\0' at the end of a string, otherwise
the final ']' won't appear in the string).

But you don't need snprintf() here, sprintf() will do since you know
in advance how long the string is going to be you're printing (at
least if you get the calculations right;-)

sprintf( curtime, "[%s]", p );

printf("%s\n", curtime);
return curtime;
}

Regards, Jens

 

[Fred Kleinschmidt]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the format of:

[Thu May 5 17:06:27 2005]

Here is my function as it stands:

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');

Several problems here.
First, what do you think "sizeof(curtime+2)" will be?
Second, curtime has been allocated for enough space for the contents of
'p' plus two characters. But you are trying to put 3 extra characters in
it ('[', ']', and '\0')
Third, you should be using sprintf, not snprintf.

 

[Emmanuel Delahaye]

Materialised wrote on 05/05/05 :

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');
printf("%s\n", curtime);
return curtime;
}

You should learn more about the meaning of sizeof. BTW, you don't need
snprintf() here, because you have calculated the size (kinda belt +
suspenser strategy, isn't it ?)

Try that

#include <stdio.h>
#include <time.h>
#include <string.h>
#include <stdlib.h>

char *currenttime (void)
{
char *curtime = NULL;
time_t t = time (NULL);
char *p = ctime (&t);

if (p != NULL)
{
{
char *e = strchr (p, '\n');

if (e != NULL)
{
*e = '\0';
}
}

curtime = malloc (strlen (p) + 2);

if (curtime != NULL)
{
sprintf (curtime, "%c%s%c", '[', p, ']');
}
}
return curtime;
}

int main (void)
{
char *s = currenttime ();

if (s != NULL)
{
printf ("currenttime = %s\n", s);
free (s), s = NULL;
}

return 0;
}

currenttime = [Thu May 05 19:26:10 2005]

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

I once asked an expert COBOL programmer, how to
declare local variables in COBOL, the reply was:
"what is a local variable?"

 

[Emmanuel Delahaye]

First problem: 'e' is an uninitialized pointer,
Ok.

so you can't store
anything where it's currently pointing to. But since 'e' is never
going to be used you can simply remove the whole line.

Huh ? What about 'e = strchr(p, '\n')' <...> *e = '\0'; ?

Ok, the way it was written was not the clearest way, but the OP wanted
to remove the trailing '\n' given by ctime(), and he has done it
correctly.

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

I once asked an expert COBOL programmer, how to
declare local variables in COBOL, the reply was:
"what is a local variable?"

 

[Emmanuel Delahaye]

(supersedes <[email protected]>)

Materialised wrote on 05/05/05 :

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');
printf("%s\n", curtime);
return curtime;
}

You should learn more about the meaning of sizeof. BTW, you don't need
snprintf() here, because you have calculated the size (kinda belt +
suspenser strategy, isn't it ?)

Try that

#include <stdio.h>
#include <time.h>
#include <string.h>
#include <stdlib.h>

char *currenttime (void)
{
char *curtime = NULL;
time_t t = time (NULL);
char *p = ctime (&t);

if (p != NULL)
{
{
char *e = strchr (p, '\n');

if (e != NULL)
{
*e = '\0';
}
}

curtime = malloc (strlen (p) + 3);

if (curtime != NULL)
{
sprintf (curtime, "%c%s%c", '[', p, ']');
}
}
return curtime;
}

int main (void)
{
char *s = currenttime ();

if (s != NULL)
{
printf ("currenttime = %s\n", s);
free (s), s = NULL;
}

return 0;
}

currenttime = [Thu May 05 19:26:10 2005]

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

..sig under repair

 

[Default User]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the

format of:


How are we supposed to divine what you consider to be the "problem" you
are having? Next time, tell us.




Brian

 

[Chris Torek]

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');
printf("%s\n", curtime);
return curtime;
}

Your problem: sizeof(curtime + 2)

This is the most obvious problem, to be sure, but not the only one.

Suppose p == NULL, so that:

if (p && ...)

does not do anything. In that case, what does:

strlen(p)

do?

Luckily, ctime() is forbidden from returning NULL, so the "p &&"
is unnecessary in the first place. We could just use:

if ((e = strchr(p, '\n')) != NULL) *e = '\0';
curtime = malloc( ... );

Then:

You want (strlen(p) + 2) instead.

This gets into the second problem, which someone else noted in a
different followup: strlen(p) + 2 lacks room for the terminating
'\0'. The malloc() argument needs to be strlen(p) + 3. This is
also the number you should pass to snprintf(). (That is, you supply
the actual number of bytes, and snprintf subtracts 1 to account
for the '\0'. Note that this is not the case for scanf field
widths, where you need to pass a number 1 byte smaller than the
buffer. The set of C functions that take or return "actual buffer
sizes", versus those that take or return "length of string not
including terminating '\0'", gets confusing. There is little if
any pattern; you must simply memorize, or look up, each function.)

In this case, of course, there is no real need to use snprintf()
at all, since curtime was presumably malloc()ed to the correct
size in the first place (except, well, "it wasn't" ).

There is a slight possibility that time() will return (time_t)-1,
meaning "the system has no idea what time it is". I have no idea
what the OP would like to happen in this case.

Finally, there are a number of ways to simplify, or at least shorten,
the code, by using various features of the Standard C Library.
Probably the best alternative is to use strftime(), but I will
illustrate a little-used feature of printf:

char *currenttime(void) {
char *curtime;
time_t t;
char *p;
size_t len;

t = time(NULL);
if (t == (time_t)-1)
... do something ...
p = ctime(&t);
len = strlen(p); /* actually, always exactly 25 */

/*
* Explanation of constants: len+1 is the number of bytes
* required for the string in p. We add 2 for the two
* brackets [], and subtract 1 for the newline we will
* remove.
*/
curtime = malloc(len + 1 + 2 - 1);
if (curtime == NULL) {
printf("out of memory\n");
exit(1);
}

/*
* We use %.*s to trim off the newline.
*/
sprintf(curtime, "[%.*s]", (int)(len - 1), p);
printf("%s\n", curtime);
return curtime;
}

The ".*" modifier in "%.*s" means "fetch an int, and use it
to limit the number of characters printed". In this case,
we truncate the last character of the string in p, which is
the unwanted newline. The cost here is relatively low since
we have already computed the length for the malloc() call.

Of course, with the comments added, the code has gotten
longer, but then, comments tend to do that.

 

[David Resnick]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the

format of:

[Thu May 5 17:06:27 2005]

Here is my function as it stands:

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';

First problem: 'e' is an uninitialized pointer, so you can't store
anything where it's currently pointing to. But since 'e' is never
going to be used you can simply remove the whole line.

That code looks OK. He is trimming off a newline. In the case
where e is dereferenced it is always initialized. Given that he needs
to figure out the lenght of p anyway this is perhaps an inefficient way
to do that but...

-David

 

[Martin Ambuhl]

I am having a problem with the following functions, I've been racking my
brains trying to figure out where I am going wrong. What I need to do is
return a formatted string with the current date and time in the format of:

[Thu May 5 17:06:27 2005]

#include <time.h>
#include <stdio.h>

#define BIGENUF 256

int main(void)
{
char nowstr[BIGENUF];
time_t t = time(0);
struct tm now;
now = *localtime(&t);
strftime(nowstr, BIGENUF, "[ %c ]", &now);
printf("%s\n", nowstr);
return 0;
}

 

[Randy Howard]

Of course, with the comments added, the code has gotten
longer, but then, comments tend to do that.

Chris, when are you going to write your own programming book?
Your examples are incredibly well written and thorough.

 

[Emmanuel Delahaye]

Randy Howard wrote on 05/05/05 :

Chris, when are you going to write your own programming book?
Your examples are incredibly well written and thorough.

Chris's site is fine if you can't wait:

http://web.torek.net/torek/c/

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"C is a sharp tool"

 

[James Dennett]

Materialised wrote on 05/05/05 :

char *currenttime(){
char *curtime;
time_t t;
char *p, *e;
t = time(NULL);
p = ctime(&t);
if (p && ((e = strchr(p, '\n')))) *e = '\0';
curtime = malloc(strlen(p)+2);
if(curtime == NULL) {
printf("out of memory\n");
exit(1);
}
snprintf(curtime, sizeof(curtime+2), "%c%s%c", '[', p, ']');
printf("%s\n", curtime);
return curtime;
}

You should learn more about the meaning of sizeof. BTW, you don't need
snprintf() here, because you have calculated the size (kinda belt +
suspenser strategy, isn't it ?)

Avoiding use of sprintf altogether isn't a bad idea; sometimes it
might be infinitesimally superior to snprintf, just as on occasion
it could be argued that a goto would actually make for good code
(I might disagree with such an argument, but that's a different
point...) but the correctness/security issues that come from using
sprintf are horrible enough to want to ban it from most uses.

(That said, for portability reasons in the real world I'd still
use a wrapper over snprintf to ensure NUL termination and to allow
for workarounds on systems lacking a native snprintf.)

-- James

 

[Jens.Toerring]

Huh ? What about 'e = strchr(p, '\n')' <...> *e = '\0'; ?

Ok, the way it was written was not the clearest way, but the OP wanted
to remove the trailing '\n' given by ctime(), and he has done it
correctly.

Yes, sorry, I somehow managed to overlook the assignment to 'e' in
the if-clause. Perhaps I should go and have my glasses checked;-)

Regards, Jens