What is wrong with this code?

P

Prabh

Hello,

I am wondering whats wrong with the following code. It is crashing on
Linux, but works fine on AIX.

The function basically get DbNm@Srvr string and then return DbNm and
Srvr back to the calling program.


#define IDFR_VAL_SIZE 100

int rmlPvtGetDbNm(char *sDb, char *sSrvr, char *sDbNm)
{
char *sTok;

memset(sDb, '\0', IDFR_VAL_SIZE);
memset(sSrvr, '\0', IDFR_VAL_SIZE);

sTok = malloc(IDFR_VAL_SIZE);

sTok = strtok( sDbNm, "@" );
strcpy(sDb, sTok);

sTok = strtok(NULL, "@" );
strcpy(sSrvr, sTok);

free(sTok);

if (memcmp(sDb, "NO-DB", 5) ==0)
{
return 1;
}
return 0;

}


with regards,

prabh
 
J

Jack Klein

Hello,

I am wondering whats wrong with the following code. It is crashing on
Linux, but works fine on AIX.

The function basically get DbNm@Srvr string and then return DbNm and
Srvr back to the calling program.


#define IDFR_VAL_SIZE 100

int rmlPvtGetDbNm(char *sDb, char *sSrvr, char *sDbNm)
{
char *sTok;

memset(sDb, '\0', IDFR_VAL_SIZE);
memset(sSrvr, '\0', IDFR_VAL_SIZE);

Where are the arrays of characters that sDb and sSrvr point to
allocated or defined? If either of them actually points to less than
100 characters, you are writing past into memory you don't own, a good
way to crash.
sTok = malloc(IDFR_VAL_SIZE);

sTok = strtok( sDbNm, "@" );

This is a memory leak. You allocate 100 characters to sTok, then
throw away the allocated memory by assigning the return value of
strtok() to it. Poof, 100 bytes leaked and gone.
strcpy(sDb, sTok);

strtok() can return a null pointer. Passing a null pointer as the
second argument to strcpy() is undefined behavior and can cause a
crash. Alternatively, if the first '@' character is more than 100
characters into sDbNm, you'll overflow
sTok = strtok(NULL, "@" );
strcpy(sSrvr, sTok);

Likewise, strtok() can return a null pointer here. Never use the
value returned by strtok() without testing for NULL.
free(sTok);

This is almost certainly what is causing your crash. You allocated
memory to sTok, then you lost the pointer to it by overwriting it with
the return value of strtok(), not once, but twice.

So now you are trying to free sTok. But the value you are passing to
free() is not the pointer value returned by malloc().
if (memcmp(sDb, "NO-DB", 5) ==0)
{
return 1;
}
return 0;

}


with regards,

prabh

--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://www.eskimo.com/~scs/C-faq/top.html
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.learn.c-c++ ftp://snurse-l.org/pub/acllc-c++/faq
 
J

James Antill

Hello,

I am wondering whats wrong with the following code. It is crashing on
Linux, but works fine on AIX.

I'm sure IBM would help you fix^H^H^H port your code.
The function basically get DbNm@Srvr string and then return DbNm and
Srvr back to the calling program.

#define IDFR_VAL_SIZE 100

int rmlPvtGetDbNm(char *sDb, char *sSrvr, char *sDbNm)
{
char *sTok;

memset(sDb, '\0', IDFR_VAL_SIZE);
memset(sSrvr, '\0', IDFR_VAL_SIZE);

sTok = malloc(IDFR_VAL_SIZE);

sTok = strtok( sDbNm, "@" );

You've just lost the memory from the malloc.
strcpy(sDb, sTok);

sTok = strtok(NULL, "@" );
strcpy(sSrvr, sTok);

free(sTok);

Now you are free'ing a random offset into sDbNm.
if (memcmp(sDb, "NO-DB", 5) ==0)
{
return 1;
}
return 0;

}

You also assume a lot of things about the sizes of the objects and don't
check any return values (and the memcmp() at the end implies to me that
the strtok()'s could fail).
 
M

Martin Ambuhl

Prabh said:
Hello,

I am wondering whats wrong with the following code. It is crashing on
Linux, but works fine on AIX.

The function basically get DbNm@Srvr string and then return DbNm and
Srvr back to the calling program.


#define IDFR_VAL_SIZE 100

int rmlPvtGetDbNm(char *sDb, char *sSrvr, char *sDbNm)
{
char *sTok;

memset(sDb, '\0', IDFR_VAL_SIZE);
memset(sSrvr, '\0', IDFR_VAL_SIZE);

sTok = malloc(IDFR_VAL_SIZE);

sTok = strtok( sDbNm, "@" );
strcpy(sDb, sTok);

sTok = strtok(NULL, "@" );
strcpy(sSrvr, sTok);

free(sTok);

if (memcmp(sDb, "NO-DB", 5) ==0)
{
return 1;
}
return 0;

}

You overwrite the value returned from malloc() not once, but three
times! No wonder free() gets confused. Even worse, there was never any
need to malloc() or free() anything. See the code below to see just how
much simpler your code could have been. Why your code "worked" before is
anyone's guess. One correct way of your code's working is to crash.


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

#define IDFR_VAL_SIZE 100

int rmlPvtGetDbNm(char *sDb, char *sSrvr, char *sDbNm)
{
char *sTok;

if ((sTok = strtok(sDbNm, "@")))
strcpy(sDb, sTok);

if ((sTok = strtok(NULL, "@")))
strcpy(sSrvr, sTok);

return (memcmp(sDb, "NO-DB", 5) == 0);
}


int main(void)
{
char input[IDFR_VAL_SIZE] = "DbNm@Srvr";
char dbnm[IDFR_VAL_SIZE], srvr[IDFR_VAL_SIZE];
printf("return value was: %d\n", rmlPvtGetDbNm(dbnm, srvr, input));
printf("dbnm: %s, srvr: %s\n", dbnm, srvr);
return 0;
}
 

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

No members online now.

Forum statistics

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

Latest Threads

Top