Weird stuff with char pointers, need someone to troubleshoot

D

dtschoepe

Hi,

I am working on a project for school and I am trying to get my head
around something weird. I have a function setup which is used to get
an input from stdin and a piece of memory is created on the heap using
malloc. This input is then run through a regex and another test. The
result is passed back as a char pointer to the program for further
processing

The first time I call this function, it performs normally. However,
after that if I call it in succession, the contents of the char array
behave strangely. Example, if I input a set of 3 numbers like "345" on
the first try, the strlen function returns the correct number 3 once
main has control of the program flow. But then if I go through the
loop again and type in 3 more numbers, the resulting array now has a
strlen of 4. Before I get the new number, I free the memory of the
char pointer that was used to hold the string. If I loop through this
over and over, the size of the strlen result changes and when I print
out the ASCII values of the string, it shows alot of weird characters
are showing up in there.

I need some help, I'm stuck. Below is the entire code I'm working with
(including printf statements for troubleshooting purposes).

David

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

void jsw_flush ( void ) /*from http://c-for-dummies.com/faq/*/
{
int ch; /* getchar returns an int */

/* Read characters until there are none left */
do
ch = getchar();
while ( ch != EOF && ch != '\n') ;
clearerr ( stdin ); /* Clear EOF state */
}

/* match input against regex */
int match(const char *string, char *pattern)
{
int status;
regex_t re;

if(regcomp(&re, pattern, REG_EXTENDED|REG_NOSUB) != 0)
{ return 0; }

status = regexec(&re, string, (size_t)0, NULL, 0);

regfree(&re);

if(status != 0)
{ return 0; }

return 1;
}

/* get input from user, discard excess input */
char * inputFlush()
{
char temp[52];
char * input = NULL;
int len = 0;
fgets(temp, sizeof(temp), stdin);
temp[strlen(temp)-1] = '\0'; /* trim off last character */
len = strlen(temp);
printf("len: %i\n", len);
input = (char *)malloc(sizeof(char) * len);
strncpy(input, temp, len);
printf("input is %s\n", input);
jsw_flush();
return input;
}


int main()
{

int error = 1, number = 0, i = 0;
char * numInput1;
char * numPattern = "^-?[0-9]{1,5}$";


/* input - numbers */
while (number != -1)
{
printf("numInput1 is: %p\n", numInput1);
printf("Enter a number: ");
numInput1 = inputFlush();
printf("numInput1 is: %p\n", numInput1);
for (i=0; i < strlen(numInput1); i++)
printf("%x\n", numInput1);
printf("Strlen(numInput1) %i\n", strlen(numInput1));
printf("numInput1 contains: %s\n", numInput1);
error = match(numInput1, numPattern); /* run regex against number
input */
printf("error: %i\n", error);
if (!error) /* num contains illegal characters */
printf("number has illegal characters\n");
if (error)
{
number = atol(numInput1); /* convert string to int */
printf("number value: %i\n", number);
if (number <= 32767 && number >= -32768) /* check to see if in
valid range */
printf("number is in valid range\n"); /* int OK move on */
else
printf("number is not in range\n"); /* bad int, get another */
}
free(numInput1);
numInput1 = NULL;
}
}
 
D

dtschoepe

Let me post a version of this without the first function that was
included in the original post. I think that one probably is not needed
for this task.

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

/* match input against regex */
int match(const char *string, char *pattern)
{
int status;
regex_t re;

if(regcomp(&re, pattern, REG_EXTENDED|REG_NOSUB) != 0)
{ return 0; }

status = regexec(&re, string, (size_t)0, NULL, 0);

regfree(&re);

if(status != 0)
{ return 0; }

return 1;
}

/* get input from user, discard excess input */
char * inputFlush()
{
char temp[52];
char * input = NULL;
int len = 0;
fgets(temp, sizeof(temp), stdin);
temp[strlen(temp)-1] = '\0';
len = strlen(temp);
printf("len: %i\n", len);
input = (char *)malloc(sizeof(char) * len);
strncpy(input, temp, len);
printf("input is %s\n", input);
return input;
}


int main()
{

int error = 0, number = 0, i = 0;
char * numInput1;
char * numPattern = "^[-+]?[0-9]{1,5}$";


/* input - numbers */
while (number != -1)
{
printf("Enter a number: ");
numInput1 = inputFlush();
for (i=0; i < strlen(numInput1); i++)
printf("%x\n", numInput1);
printf("strlen(numInput1) %i\n", strlen(numInput1));
error = match(numInput1, numPattern);
printf("error: %i\n", error);
if (!error)
printf("number has illegal characters\n");
if (error)
{
number = atol(numInput1);
printf("number value: %i\n", number);
if (number <= 32767 && number >= -32768)
printf("number is in valid range\n");
else
printf("number is not in range\n");
}
free(numInput1);
numInput1 = NULL;
}
}
 
I

Ico

Hi,

I am working on a project for school and I am trying to get my head
around something weird. I have a function setup which is used to get
an input from stdin and a piece of memory is created on the heap using
malloc. This input is then run through a regex and another test. The
result is passed back as a char pointer to the program for further
processing

The first time I call this function, it performs normally. However,
after that if I call it in succession, the contents of the char array
behave strangely. Example, if I input a set of 3 numbers like "345" on
the first try, the strlen function returns the correct number 3 once
main has control of the program flow. But then if I go through the
loop again and type in 3 more numbers, the resulting array now has a
strlen of 4. Before I get the new number, I free the memory of the
char pointer that was used to hold the string. If I loop through this
over and over, the size of the strlen result changes and when I print
out the ASCII values of the string, it shows alot of weird characters
are showing up in there.

I need some help, I'm stuck. Below is the entire code I'm working with
(including printf statements for troubleshooting purposes).

David

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

(people will complain about this header on comp.lang.c)
void jsw_flush ( void ) /*from http://c-for-dummies.com/faq/*/
{
int ch; /* getchar returns an int */

[snipped somde code]
/* get input from user, discard excess input */
char * inputFlush()
{
char temp[52];
char * input = NULL;
int len = 0;
fgets(temp, sizeof(temp), stdin);
temp[strlen(temp)-1] = '\0'; /* trim off last character */
len = strlen(temp);
printf("len: %i\n", len);
input = (char *)malloc(sizeof(char) * len);

Rethink what you are doing here. C-strings need a terminating 0.
 
S

santosh

Let me post a version of this without the first function that was
included in the original post. I think that one probably is not needed
for this task.

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

/* match input against regex */
int match(const char *string, char *pattern)
{
int status;
regex_t re;

if(regcomp(&re, pattern, REG_EXTENDED|REG_NOSUB) != 0)
{ return 0; }

status = regexec(&re, string, (size_t)0, NULL, 0);

regfree(&re);

if(status != 0)
{ return 0; }

return 1;
}

/* get input from user, discard excess input */
char * inputFlush()
{
char temp[52];
char * input = NULL;
int len = 0;
fgets(temp, sizeof(temp), stdin);

Check the fgets call. It can return NULL when an error occurs or end-
of-file is encountered.
temp[strlen(temp)-1] = '\0';

Why? fgets always terminates it's output with a null character. You're
overwriting the last non-null character here.
len = strlen(temp);

strlen returns the length of a string *without* counting it's
terminating null character.
printf("len: %i\n", len);
input = (char *)malloc(sizeof(char) * len);

Don't cast the return value of malloc. It can hide certain mistakes.
I'd write the above as:

input = malloc(len+1 * sizeof *input);

Notice that I added one to len for accomodating a null character.
Otherwise what's left isn't a C string.
strncpy(input, temp, len);

Supply len+1 here too.
printf("input is %s\n", input);

The %s specifier expects a null terminated string. You're passing an
array of char without a null terminator.
return input;
}


int main()
{

int error = 0, number = 0, i = 0;
char * numInput1;
char * numPattern = "^[-+]?[0-9]{1,5}$";


/* input - numbers */
while (number != -1)
{
printf("Enter a number: ");

Terminate output with a newline or call fflush(stdout) immediatly
afterwards.
numInput1 = inputFlush();
for (i=0; i < strlen(numInput1); i++)
printf("%x\n", numInput1);
printf("strlen(numInput1) %i\n", strlen(numInput1));


Why not compute the string length once and store it, instead of
calling strlen again and again?
error = match(numInput1, numPattern);
printf("error: %i\n", error);
if (!error)
printf("number has illegal characters\n");
if (error)
{
number = atol(numInput1);

I'd advise you to use strtol here, since it has better error checking
and reporting.
printf("number value: %i\n", number);
if (number <= 32767 && number >= -32768)

Why code in this non-portable construct. Just compare with INT_MAX and
INT_MIN defined in limits.h. The range of integers, (and floats), will
vary from platform to platform. Each implementation defines the ranges
in limits.h and float.h. Using those macros is more robust than
hardcoding values specific for a platform.
printf("number is in valid range\n");
else
printf("number is not in range\n");
}
free(numInput1);
numInput1 = NULL;
}

Return a value from main with a return statement.
 
D

dtschoepe

[snipped some code]
/* get input from user, discard excess input */
char * inputFlush()
{
char temp[52];
char * input = NULL;
int len = 0;
fgets(temp, sizeof(temp), stdin);
temp[strlen(temp)-1] = '\0'; /* trim off last character */
len = strlen(temp);
printf("len: %i\n", len);
input = (char *)malloc(sizeof(char) * len);

Rethink what you are doing here. C-strings need a terminating 0.

Thanks, that fixed it for me.

David
 
F

Fred Kleinschmidt

santosh said:
Let me post a version of this without the first function that was
included in the original post. I think that one probably is not needed
for this task.

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

/* match input against regex */
int match(const char *string, char *pattern)
{
int status;
regex_t re;

if(regcomp(&re, pattern, REG_EXTENDED|REG_NOSUB) != 0)
{ return 0; }

status = regexec(&re, string, (size_t)0, NULL, 0);

regfree(&re);

if(status != 0)
{ return 0; }

return 1;
}

/* get input from user, discard excess input */
char * inputFlush()
{
char temp[52];
char * input = NULL;
int len = 0;
fgets(temp, sizeof(temp), stdin);

Check the fgets call. It can return NULL when an error occurs or end-
of-file is encountered.
temp[strlen(temp)-1] = '\0';

Why? fgets always terminates it's output with a null character. You're
overwriting the last non-null character here.
len = strlen(temp);

strlen returns the length of a string *without* counting it's
terminating null character.
printf("len: %i\n", len);
input = (char *)malloc(sizeof(char) * len);

Don't cast the return value of malloc. It can hide certain mistakes.
I'd write the above as:

input = malloc(len+1 * sizeof *input);

Notice that I added one to len for accomodating a null character.
Otherwise what's left isn't a C string.

No, you didn't. You added (1 * sizeof(*input)) to len
strncpy(input, temp, len);

Supply len+1 here too.
printf("input is %s\n", input);

The %s specifier expects a null terminated string. You're passing an
array of char without a null terminator.
return input;
}


int main()
{

int error = 0, number = 0, i = 0;
char * numInput1;
char * numPattern = "^[-+]?[0-9]{1,5}$";


/* input - numbers */
while (number != -1)
{
printf("Enter a number: ");

Terminate output with a newline or call fflush(stdout) immediatly
afterwards.
numInput1 = inputFlush();
for (i=0; i < strlen(numInput1); i++)
printf("%x\n", numInput1);
printf("strlen(numInput1) %i\n", strlen(numInput1));


Why not compute the string length once and store it, instead of
calling strlen again and again?
error = match(numInput1, numPattern);
printf("error: %i\n", error);
if (!error)
printf("number has illegal characters\n");
if (error)
{
number = atol(numInput1);

I'd advise you to use strtol here, since it has better error checking
and reporting.
printf("number value: %i\n", number);
if (number <= 32767 && number >= -32768)

Why code in this non-portable construct. Just compare with INT_MAX and
INT_MIN defined in limits.h. The range of integers, (and floats), will
vary from platform to platform. Each implementation defines the ranges
in limits.h and float.h. Using those macros is more robust than
hardcoding values specific for a platform.
printf("number is in valid range\n");
else
printf("number is not in range\n");
}
free(numInput1);
numInput1 = NULL;
}

Return a value from main with a return statement.
 
K

Keith Thompson

santosh said:
(e-mail address removed) wrote: [...]
fgets(temp, sizeof(temp), stdin);

Check the fgets call. It can return NULL when an error occurs or end-
of-file is encountered.
temp[strlen(temp)-1] = '\0';

Why? fgets always terminates it's output with a null character. You're
overwriting the last non-null character here.
[...]

The string produced by fgets() typically ends in a '\n' character
(just before the '\0'). Probably the OP wanted the string without the
'\n' (<OT>like Perl's "chomp"</OT>). But he should check that the
character is a '\n' before stepping on it; it won't contain a '\n' if
fgets() reads the specified number of characters before reaching the
end of the input line.
 

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,734
Messages
2,569,441
Members
44,832
Latest member
GlennSmall

Latest Threads

Top