Go Premium for a chance to win a PS4. Enter to Win

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 711
  • Last Modified:

Global pointer problems

Hello List,
When the program reaches line 90, I am not sure why I would get no content in ctxt->sdiUser and the strange "s" in ctxt->sdiRequest, as shown in the snapshot in the png file attached.
Just in case of problems accessing, I have attached the completed code as a txt file as well.
The code compiles with no errors.

Arthur
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#define AR_MAX_DEFAULT_SIZE 255
 
typedef struct
{
int iInitRun;
char *sdiRequest;
char *sdiResponse;
char *sdiUser;
char *sdiPwd;
char *sdiQ;
char *sdiBrokerList;
} Context;
 
void CreateInstance(void **);
void GLEWF(void **);
void talker(void **);
void onMessage(void **);
void Init(void **);
void DeleteInstance(void **);
void PrintContext(void **);
 
void PrintContext(void **object)
{
    Context *printCtxt = (Context *)object;
//    if((int *)printCtxt->iInitRun != 0)
//    fprintf(stdout,"InitRun - %d\n",printCtxt->iInitRun);
    if(printCtxt->sdiRequest != NULL)
    fprintf(stdout,"sdiRequest - %s\n",printCtxt->sdiRequest);
    if(printCtxt->sdiResponse != NULL)
    fprintf(stdout,"sdiResponse - %s\n",printCtxt->sdiResponse);
    if(printCtxt->sdiUser != NULL)
    fprintf(stdout,"sdiUser - %s\n",printCtxt->sdiUser);
    if(printCtxt->sdiPwd != NULL)
    fprintf(stdout,"sdiPwd - %s\n",printCtxt->sdiPwd);
    if(printCtxt->sdiQ != NULL)
    fprintf(stdout,"sdiQ - %s\n",printCtxt->sdiQ);
    if(printCtxt->sdiBrokerList != NULL)
    fprintf(stdout,"sdiBrokerList - %s",printCtxt->sdiBrokerList);
}
 
void CreateInstance(void **object)
{
  Context *context;
 
  if (object != NULL)
    *object = NULL;
 
  context = (Context *) malloc(sizeof(Context));
 
  if (context == NULL)
  {
    fprintf(stdout,"CI:Error memory allocation for Context");
    return;
  }
 
 
  *object = (void *) context;
  context->sdiUser = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  context->sdiPwd = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  context->sdiQ = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  context->sdiBrokerList = (char *)malloc(AR_MAX_DEFAULT_SIZE);
 
  strcpy(context->sdiUser,"CI:User");
  context->sdiUser[AR_MAX_DEFAULT_SIZE]='\0';
  strcpy(context->sdiPwd,"CI:Pwd");
  context->sdiPwd[AR_MAX_DEFAULT_SIZE]='\0';
  strcpy(context->sdiQ,"CI:Q");
  context->sdiQ[AR_MAX_DEFAULT_SIZE]='\0';
  strcpy(context->sdiBrokerList,"CI:Broker");
  context->sdiBrokerList[AR_MAX_DEFAULT_SIZE]='\0';
 
  //PrintContext(*object);
 
  Init(*object);
}
 
void DeleteInstance(void **object)
{
    Context       *context = (Context *) object;
    PrintContext(*object);
    free(context);
}
 
void GLEWF(void **object)
{
    Context *ctxt = (Context *)object;
    ctxt->sdiRequest = (char *)malloc(AR_MAX_DEFAULT_SIZE);
    strcpy(ctxt->sdiRequest,"From GLEWF:Hello World");
    ctxt->sdiRequest[AR_MAX_DEFAULT_SIZE]='\0';
    PrintContext(*object);
    talker(*object);
    free(ctxt->sdiRequest);
}
 
void talker(void **object)
{
    Context *talkCtxt = (Context *) object;
    fprintf(stdout,"talker:%s\n",talkCtxt->sdiRequest);
    strcpy(talkCtxt->sdiResponse, "From talker:Response");
}
 
void Init(void **object)
{
    Context *initCtxt = (Context *)object;
    fprintf(stdout,"Init:%s\n",initCtxt->sdiUser);
    strcpy(initCtxt->sdiUser,"From Init:Reuben");
}
 
int main()
{
    void **obj;
    CreateInstance(*obj);
    GLEWF(*obj);
    DeleteInstance(*obj);
    //printf("Hello world!\n");
    return 0;
}

Open in new window

ptr-problem.png
main.txt
0
software_architect
Asked:
software_architect
  • 5
  • 5
  • 3
  • +1
1 Solution
 
rowansmithCommented:
Why do you have so much referencing and deferencing?

On line 114 you declare **obj but never define an array of objects for this to point to.

then in line 115 you send the first pointer that was never defined....

the reason this thing compiles at all is because of all the casting you are doing.  You should only cast where you are specifically changing from one datatype to another.

I can not see any requirement for an array of objects in your code, so you should not be using ** at all....

Or am I missing something.
0
 
sunnycoderCommented:
ctxt->sdiUser is an uninitialized variable. It can contain any value in this case it contains 0x05 which is out of bounds for your program.

ctxt->sdiRequest has been initialized with address of a valid memory location but that memory location has not been initialized. Printing sdiRequest at that point gives you whatever the memory contains - in this case it is string "s" ... it could well have been any string.

It is like putting address 1000 in sdiRequest but not putting anything at location 1000. Even though you are printing contents of valid memory location, the contents themselves are "junk"
0
 
software_architectAuthor Commented:
Thanks Rowan.
I put this code together since this is part of a library code that is loaded into a daemon on Solaris. the daemon calls each of the functions for specific actions and also maintains the object pointer throughout the collection of functions.

The first time the library is loaded into the daemon, it calls the CreateInstance function:
CreateInstance(void **object,...)

when a "Search" Action is performed the daemon calls the GLEWF function:
GLEWF(void **object,...)

In order to have the code function in a thread safe manner, we were told to declare a global structure like Context, have all the global variables and pointers within it and then have the following code in CreateInstance():

       context = (Context *) malloc(sizeof(Context));
 
      /* initialize variables */
      context->...;

       *object = (void *) context;

For a particular action named - "Search", the daemon calls GLEWF(). here we were told to use the following code:
       Context       *context = (Context *) object;
and then can refer to the structure elements by using - context->element

So the object pointer is actually maintained by the daemon service. I am trying to mimic that behaviour so I can test the rest of the code like the function call to Init from within CreateInstance which just uses the last four structure elements and setting sdiRequest element from GLEWF() and using that within talker() where I set sdiResponse element. This way I just use the object pointer to use/set elements and make sure that other functions can use the values being set into these elements.

Arthur
0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
sunnycoderCommented:
    void **obj;
    CreateInstance(*obj);
    GLEWF(*obj);

>void CreateInstance(void **object)

    CreateInstance(obj);
    GLEWF(obj);

CreateInstance expects a pointer to pointer while you are passing it a pointer. Same for GLEFW
You are not checking value returned by malloc which can be dangerous.
0
 
rowansmithCommented:
The lines that do this have no value

object->sdiUser[AR_MAX_DEFAULT_SIZE]='\0';

The strcpy already appends the NULL at the end of the string, this puts it at the end of the array which is really quite pointless.

See the code below, it does what your program seems to be designed to do....
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#define AR_MAX_DEFAULT_SIZE 255
 
typedef struct
{
int iInitRun;
char *sdiRequest;
char *sdiResponse;
char *sdiUser;
char *sdiPwd;
char *sdiQ;
char *sdiBrokerList;
} Context;
 
Context *CreateInstance(Context *);
void GLEWF(Context *);
void talker(Context *);
void onMessage(void **);
void Init(Context *);
void DeleteInstance(Context *);
void PrintContext(Context *);
 
void PrintContext(Context *printCtxt)
{
    
    if(printCtxt->iInitRun != 0)
		fprintf(stdout,"InitRun - %d\n",printCtxt->iInitRun);
    if(printCtxt->sdiRequest)
		fprintf(stdout,"sdiRequest - %s\n",printCtxt->sdiRequest);
    if(printCtxt->sdiResponse)
		fprintf(stdout,"sdiResponse - %s\n",printCtxt->sdiResponse);
    if(printCtxt->sdiUser)
		fprintf(stdout,"sdiUser - %s\n",printCtxt->sdiUser);
    if(printCtxt->sdiPwd)
		fprintf(stdout,"sdiPwd - %s\n",printCtxt->sdiPwd);
    if(printCtxt->sdiQ)
		fprintf(stdout,"sdiQ - %s\n",printCtxt->sdiQ);
    if(printCtxt->sdiBrokerList)
		fprintf(stdout,"sdiBrokerList - %s",printCtxt->sdiBrokerList);
}
 
Context *CreateInstance(void)
{
  // this is the only time you should need to cast
  Context *object = (Context *) malloc(sizeof(Context));
 
  if (object == NULL)
  {
    fprintf(stdout,"CI:Error memory allocation for Context");
    return(NULL);
  }
 
  // Set everything to NULL so that printcontext does not barf on NON Nulls.
	object->sdiRequest = NULL;
	object->sdiResponse = NULL;
	object->sdiUser = NULL;
	object->sdiPwd = NULL;
	object->sdiQ = NULL;
	object->sdiBrokerList = NULL;
 
  object->sdiUser = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiPwd = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiQ = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiBrokerList = (char *)malloc(AR_MAX_DEFAULT_SIZE);
 
  strcpy(object->sdiUser,"CI:User");
  strcpy(object->sdiPwd,"CI:Pwd");
  strcpy(object->sdiQ,"CI:Q");
  strcpy(object->sdiBrokerList,"CI:Broker");
   
  //PrintContext(*object);
 
  Init(object);
 
  return(object);
}
 
//void DeleteInstance(void *object)
//{
//    Context       *context = (Context *) object;
//    PrintContext(object);
//    free(context);
//}
// 
void GLEWF(Context *ctxt)
{    
    ctxt->sdiRequest = (char *)malloc(AR_MAX_DEFAULT_SIZE);
    strcpy(ctxt->sdiRequest,"From GLEWF:Hello World");
    PrintContext(ctxt);
    talker(ctxt);
    free(ctxt->sdiRequest);
	ctxt->sdiRequest = NULL;
}
// 
void talker(Context *talkCtxt)
{
    fprintf(stdout,"talker:%s\n",talkCtxt->sdiRequest);
    
	// sdiResponse has not been malloc'd do not do this yet...
	// strcpy(talkCtxt->sdiResponse, "From talker:Response");
}
// 
void Init(Context *object)
{
    Context *initCtxt = object;
    fprintf(stdout,"Init:%s\n",initCtxt->sdiUser);
    strcpy(initCtxt->sdiUser,"From Init:Reuben");
}
 
 
int _tmain(int argc, _TCHAR* argv[])
{
	Context *obj = NULL;
    obj = CreateInstance();
	if(!obj)
		return 1;
    GLEWF(obj);
    //DeleteInstance(obj);
	// Delete the Instance and clean up memory
	// You actually have to free all the stuff that you malloc'd e.g., all the char *'s
	// you can not just free the obj
	
    free(obj);
	// the above has only free'd up the memory taken up by obj
	// you also need to free up any memory allocation you made
	// for each of the char's.
 
    //printf("Hello world!\n");
    return 0;
}

Open in new window

0
 
software_architectAuthor Commented:
sunny,
You're right about the sdiRequest not being set.
What about ctxt->sdiUser? sdiUser, before GLEWF(), has been initialized to a memory location and value in CreateInstance(), where I am assuming that it is at a global level and should be available to all other functions. Or am i  mistaken?
0
 
rowansmithCommented:
Ok I can understand then why you need the casting...

You should be able to achieve what you want using my code above.
0
 
software_architectAuthor Commented:
Thanks rowan for the code changes
Sorry! I forgot to mention that the CreateInstance() and GLEWF() functions' params and return params can't be changed. They are part of the fixed framework for the plugin library.
0
 
sunnycoderCommented:
you are right in saying it should have been available to all as a global but you passed void * instead of void ** to CreateInstance(). As a result, the function puts the pointer at **object and not *object which the rest of your program expects.

Infact
    void **obj;
    CreateInstance(*obj); ...... *obj does not point to any valid location.

void CreateInstance(void **object)
{
  Context *context;
 
  if (object != NULL)
    *object = NULL;      ---> this assignment could well have crashed.
0
 
Infinity08Commented:
A few more problems :

1) You allocate memory for AR_MAX_DEFAULT_SIZE characters for all members, for example :

          context->sdiUser = (char *)malloc(AR_MAX_DEFAULT_SIZE);

    But then you set the character at index AR_MAX_DEFAULT_SIZE :

          context->sdiUser[AR_MAX_DEFAULT_SIZE]='\0';

    which is out of bounds, because array indexing starts at 0, so the last character is at index (AR_MAX_DEFAULT_SIZE - 1).


2) You use strcpy very often. However, you are dealing with limited length strings, so it's better to use strncpy to avoid buffer overflows :

          http://www.cplusplus.com/reference/clibrary/cstring/strncpy.html

    Notice that strncpy takes an extra parameter which says the max. amount of characters to be copied.



3) In DeleteInstance, you simply do a free of the entire object :

          free(context);

    That creates quite a memory leak ... You have to free all the members (sdiUser, etc.) first, and then the object.


4) All of your functions take a void** as parameter - however, you're not always treating it as a pointer-to-pointer. For example :

          void PrintContext(void **object)
          {
              Context *printCtxt = (Context *)object;

    You're casting a pointer-to-pointer-to-void to a pointer-to-Context. Are you sure that's what you want to do ?


5) In PrintContext, you check whether a string is NULL before printinf it :

          if(printCtxt->sdiRequest != NULL)

    That's good, but make sure that sdiRequest (and the other members) are set to NULL when creating the struct (ie. before allocating memory), and also after de-allocating sdiRequest (and the other members).
0
 
rowansmithCommented:
Here is an example of your requirement for the void ** ...

You should be able to do the rest .. sory it is just not worth enough points for me write it all :-)

-Rowan
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#define AR_MAX_DEFAULT_SIZE 255
 
typedef struct
{
int iInitRun;
char *sdiRequest;
char *sdiResponse;
char *sdiUser;
char *sdiPwd;
char *sdiQ;
char *sdiBrokerList;
} Context;
 
void CreateInstance(void **);
void PrintContext(Context *);
 
void PrintContext(Context *printCtxt)
{
    
    if(printCtxt->iInitRun != 0)
		fprintf(stdout,"InitRun - %d\n",printCtxt->iInitRun);
    if(printCtxt->sdiRequest)
		fprintf(stdout,"sdiRequest - %s\n",printCtxt->sdiRequest);
    if(printCtxt->sdiResponse)
		fprintf(stdout,"sdiResponse - %s\n",printCtxt->sdiResponse);
    if(printCtxt->sdiUser)
		fprintf(stdout,"sdiUser - %s\n",printCtxt->sdiUser);
    if(printCtxt->sdiPwd)
		fprintf(stdout,"sdiPwd - %s\n",printCtxt->sdiPwd);
    if(printCtxt->sdiQ)
		fprintf(stdout,"sdiQ - %s\n",printCtxt->sdiQ);
    if(printCtxt->sdiBrokerList)
		fprintf(stdout,"sdiBrokerList - %s",printCtxt->sdiBrokerList);
}
 
void CreateInstance(void **void_object)
{
 Context **pp_obj = (Context **)void_object;
 
 Context *p_obj = *pp_obj;
 
 Context *object = p_obj;
 
  // Set everything to NULL so that printcontext does not barf on NON Nulls.
	object->sdiRequest = NULL;
	object->sdiResponse = NULL;
	object->sdiUser = NULL;
	object->sdiPwd = NULL;
	object->sdiQ = NULL;
	object->sdiBrokerList = NULL;
 
  object->sdiUser = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiPwd = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiQ = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiBrokerList = (char *)malloc(AR_MAX_DEFAULT_SIZE);
 
  strcpy(object->sdiUser,"CI:User");
  strcpy(object->sdiPwd,"CI:Pwd");
  strcpy(object->sdiQ,"CI:Q");
  strcpy(object->sdiBrokerList,"CI:Broker");
   
  PrintContext(object);
 
}
 
 
int _tmain(int argc, _TCHAR* argv[])
{
	Context **pp_obj = NULL;
	Context *p_object = (Context *) malloc(sizeof(Context));
    pp_obj = &p_object;
 
    CreateInstance((void **)pp_obj);
 
    return 0;
}

Open in new window

0
 
rowansmithCommented:
And here is another example where the malloc is done inside CreateInstance...
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#define AR_MAX_DEFAULT_SIZE 255
 
typedef struct
{
int iInitRun;
char *sdiRequest;
char *sdiResponse;
char *sdiUser;
char *sdiPwd;
char *sdiQ;
char *sdiBrokerList;
} Context;
 
void CreateInstance(void **);
void PrintContext(Context *);
 
void PrintContext(Context *printCtxt)
{
    
    if(printCtxt->iInitRun != 0)
		fprintf(stdout,"InitRun - %d\n",printCtxt->iInitRun);
    if(printCtxt->sdiRequest)
		fprintf(stdout,"sdiRequest - %s\n",printCtxt->sdiRequest);
    if(printCtxt->sdiResponse)
		fprintf(stdout,"sdiResponse - %s\n",printCtxt->sdiResponse);
    if(printCtxt->sdiUser)
		fprintf(stdout,"sdiUser - %s\n",printCtxt->sdiUser);
    if(printCtxt->sdiPwd)
		fprintf(stdout,"sdiPwd - %s\n",printCtxt->sdiPwd);
    if(printCtxt->sdiQ)
		fprintf(stdout,"sdiQ - %s\n",printCtxt->sdiQ);
    if(printCtxt->sdiBrokerList)
		fprintf(stdout,"sdiBrokerList - %s",printCtxt->sdiBrokerList);
}
 
void CreateInstance(void **void_object)
{
 Context **pp_obj = (Context **)void_object;
 *pp_obj = (Context *) malloc(sizeof(Context));
 
 Context *p_obj = *pp_obj;
 Context *object = p_obj;
 
  // Set everything to NULL so that printcontext does not barf on NON Nulls.
	object->sdiRequest = NULL;
	object->sdiResponse = NULL;
	object->sdiUser = NULL;
	object->sdiPwd = NULL;
	object->sdiQ = NULL;
	object->sdiBrokerList = NULL;
 
  object->sdiUser = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiPwd = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiQ = (char *)malloc(AR_MAX_DEFAULT_SIZE);
  object->sdiBrokerList = (char *)malloc(AR_MAX_DEFAULT_SIZE);
 
  strcpy(object->sdiUser,"CI:User");
  strcpy(object->sdiPwd,"CI:Pwd");
  strcpy(object->sdiQ,"CI:Q");
  strcpy(object->sdiBrokerList,"CI:Broker");
   
  PrintContext(object);
 
}
 
 
int _tmain(int argc, _TCHAR* argv[])
{
	Context **pp_obj = NULL; // Create the pointer to the pointer
	Context *p_object = NULL;  // Create the Pointer to the Context Struct
	pp_obj = &p_object; // Set the pp to point to the Context Struct
 
    CreateInstance((void **)pp_obj);
 
    return 0;
}

Open in new window

0
 
software_architectAuthor Commented:
Thanks for all your help. I was able to put together the code I needed using pointers from all of you.
Points go to rowansmith.
0
 
software_architectAuthor Commented:
I attached the finalized code.

Clarification:
==========
The plugin daemon has the CreateInstance(void **), GLEWF(void *) and DeleteInstance(void *) function definitions fixed. The following are functions that are custom defined that needed to use the global object pointer.
void talker(void *);
void onMessage(void *);
void Init(void *);
void PrintContext(void *);

main-final.txt
0

Featured Post

Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

  • 5
  • 5
  • 3
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now