?
Solved

pointer, c++

Posted on 2011-10-11
19
Medium Priority
?
796 Views
Last Modified: 2012-05-12
short *Version;      

I have to pass value 1 in the version variable which is of datatype short*

int p = 1;
requestHeader->Version = (short*) &p;

Is this correct way of passing the value 1, Please let me know!

0
Comment
Question by:rbhargaw
  • 5
  • 4
  • 4
  • +3
17 Comments
 
LVL 40

Accepted Solution

by:
evilrix earned 668 total points
ID: 36949926
That will set the pointer Version to point to an integer of value p.

Whilst this will compile it isn't strictly valid C++ since it breaks the rules of strict aliasing.
0
 
LVL 31

Assisted Solution

by:Zoppo
Zoppo earned 668 total points
ID: 36949932
Hi rbhargaw,

hard to say what is meant here since I have no idea what 'requestHeader' is.

What you do is assigning a pointer to 'p' to 'requestHeader->Version' - this is dangerous at all since the pointer to 'p' may become invalid as soon as 'p' is removed from the stack when the function's scope is left.

If the 'requestHeader->Version' itself is a pointer then it depends on whether that pointer already points to some allocated memory. If the pointer isn't yet initialized a appropriate code fragment could look like this:
int p = 1;
requestHeader->Version = new short;
*requestHeader->Version = p;

Open in new window

If this doesn't help please give some more detailed info including real code you're using.

ZOPPO
0
 
LVL 5

Expert Comment

by:Kelaros
ID: 36949955
Yes that will work correctly.

But do you need Version to be a pointer?  I normally reserve pointers for classes and structs... gets confusing to use them for regular data types.
0
Veeam Disaster Recovery in Microsoft Azure

Veeam PN for Microsoft Azure is a FREE solution designed to simplify and automate the setup of a DR site in Microsoft Azure using lightweight software-defined networking. It reduces the complexity of VPN deployments and is designed for businesses of ALL sizes.

 
LVL 40

Expert Comment

by:evilrix
ID: 36950053
>> Yes that will work correctly.
Um, actually the behaviour is undefined so you can't assert that.

The C++ standard only allows you to alias a different type using a char pointer and a void pointer (because a void pointer is defined to "have the same representation and alignment requirements as a pointer to a character type.  Other pointer types need not have the same representation or alignment requirements.".

Quoted from the standard:

An object shall have its stored value accessed only by an lvalue
that has one of the following types: /28/

 * the declared type of the object,

 * a qualified version of the declared type of the object,

 * a type that is the signed or unsigned type corresponding to the
   declared type of the object,

 * a type that is the signed or unsigned type corresponding to a
   qualified version of the declared type of the object,

 * an aggregate or union type that includes one of the aforementioned
   types among its members (including, recursively, a member of a
   subaggregate or contained union), or

 * a character type.  
0
 

Author Comment

by:rbhargaw
ID: 36950200
We are interacting with an webservice which is currently working in production. Now the changes is to add 'version' in the request header with the value of 1, so I am trying to pass value 1 so that webservice . I just wanted to make sure that I am passing the values correctly before asking the vndor to look into the issue as I am unable to get responses back from webservice

class ns1__RequestHeader
{
public:

    short*                               Version                     0;      
}

Let me know if this helps.
0
 
LVL 4

Expert Comment

by:stachenov
ID: 36950616
This all depends on how the field is going to be used. Some tips:

1. Don't assign pointer to a different type via a cast, like you did. It is asking for trouble and completely unnecessary. Avoid any casts if possible.

2. Consider the variable scope. If it's local, then you'll have a dangling pointer when it goes out of scope. If it's heap allocated with new, then you could have a memory leak if you don't delete it later.

3. Don't set the pointer to point to constant (more casts avoidance) as it may get assigned somewhere later.

Ideally, you should consult your API documentation. It doesn't make sense to have a pointer field just to pass some value to it. A regular short/int would do just fine. If there is a pointer, there should be a reason for it, and the correct usage would depend on that reason.
0
 

Author Comment

by:rbhargaw
ID: 36952417
Stachenov, the pointer for short is the client decision as its their webservice, so we cannot change that ..

somehow the soap is giving error if I pasd the below code...
int p = 1;
requestHeader->Version = (short*) &p;

How do I pass the 1 value to the request header?
0
 

Author Comment

by:rbhargaw
ID: 36952513
short nValue1 = 1;

short *pnPtr1 = &nValue1;
      requestHeader->Version = pnPtr1 ;
      delete pnPtr1;

Trying this but getting heap error but it ran once!
0
 
LVL 4

Assisted Solution

by:stachenov
stachenov earned 664 total points
ID: 36953717
You a pointer, but then immediately delete it, so it becomes a dangling pointer. That just makes no sense. Try this:
 
static short version = 1;
requestHeader->Version = &version;

Open in new window

At least it won't cause any memory errors, UB or problems with the variable going out of scope. It could still give you trouble if different objects try to modify this variable concurrently, but you'll have to figure out how this field is used and find a proper place to store your short for that purpose.
0
 
LVL 35

Expert Comment

by:sarabande
ID: 36953962
when the short* is a client decision you should allocate memory for the short on the heap with new like Zoppo showed. it then is in the responsibility of the client to free (delete) that memory after use (for example by adding a destructor to the struct).

the shortest code to do so (properly) is

if (requestHeader->Version != NULL)
   *requestHeader->Version = 1;
else
   requestHeader->Version = new short(1);

Open in new window


note, the if statement expects the requestHeader is a valid pointer pointing to a well-initialized object of class ns1__RequestHeader. if the class ns1__RequestHeader is not more than that you posted - means it has no constructor and it is not granted that member pointer Version was initialised to NULL, then you should omit the if statement and only use the else part. however, that is not likely to be called a good programming style and probably would lead to memory leaks.

Sara
0
 
LVL 40

Expert Comment

by:evilrix
ID: 37066422
Um, the selected answer doesn't answer the original question, which I answered with my very first comment (no, it is not correct because the cast you are performing has an unspecified outcome).
0
 

Author Comment

by:rbhargaw
ID: 37070565
I am using like this. Is this not correct?

      short  nValue1=1;
      short *pnPtr2 = 0;

      pnPtr2 = new short;

      if (pnPtr2 == 0)
      {
            AfxMessageBox( "Error: memory could not be allocated");
      }
      else
      {
            pnPtr2 = &nValue1;
            requestHeader->Version = pnPtr2;
      }
0
 
LVL 4

Expert Comment

by:stachenov
ID: 37071170
No, this is definitely not correct.

First, you allocate a new short. This is somewhat correct, although it rarely makes sense.

Then you check for NULL, although most modern implementations just throw bad_cast instead of returning NULL. But checking doesn't hurt anyway, so this part is somewhat "correct" too.

Then, assuming the allocation was successful, you immediately forget about the newly allocated pointer by assigning it the address of nValue1 instead. Why did you allocate it then? This part is definitely not correct and will lead to memory leaks.

In the end, you've got one useless and uninitialized short hanging somewhere in the heap, and a pointer to the a variable, which will become invalid as soon as the variable goes out of scope. Since we have no idea when that happens in your particular case, or how and when the pointer is going to be used, it is hard to say for sure whether this part is correct or not.

I'd suggest you read some good book about memory management in C/C++ in general. It seems that you're confusing stack, static and heap allocations. You need to understand how all of them work if you don't want to run into a lot of troubles with pointers, which is surprisingly easy in these languages even for an experienced developer.
0
 
LVL 40

Expert Comment

by:evilrix
ID: 37071808
Why is version defined as a pointer to a short and not just a short? That is the cause of all your woes. I can't really see a sensible reason for wanting to reference the memory location of the original value unless, of course, this is actually going to be set by dereferencing requestHeader->Version at some point in the future.

Anyway, in so far as your posted code goes you are making your life very hard indeed, see below...

requestHeader->Version = new short(1); // assuming this needs to be heap allocated

... stuff happens

delete requestHeader->Version; // if you don't delete this it will leak memory!

Open in new window

0
 
LVL 35

Expert Comment

by:sarabande
ID: 37081321
the accepted answer has the advantage that the pointer keeps valid beyond the current scope and doesn't need to be freed (deleted) later.

it doesn't directly answer the question but as it corrects the possibly dangerous code by defining a static short instead of an int, it is a good solution in my opinion.
 
Sara
0
 
LVL 4

Expert Comment

by:stachenov
ID: 37081924
Yes, that was exactly my intent, to provide a workaround that is as safe as possible given that the context of the question still remains unknown. However, evilrix'es answer was addressing the question more directly, and I agree that it better answers the question itself. Moreover, he also provided a quote from the standard with relevant and useful information. Given that, I'd be happy if those points were given to him instead.
0
 
LVL 40

Expert Comment

by:evilrix
ID: 37082045
As far as the question goes the following posts answer it.

http:#a36949926
http:#a36950053
0

Featured Post

How to Use the Help Bell

Need to boost the visibility of your question for solutions? Use the Experts Exchange Help Bell to confirm priority levels and contact subject-matter experts for question attention.  Check out this how-to article for more information.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Many modern programming languages support the concept of a property -- a class member that combines characteristics of both a data member and a method.  These are sometimes called "smart fields" because you can add logic that is applied automaticall…
This article shows you how to optimize memory allocations in C++ using placement new. Applicable especially to usecases dealing with creation of large number of objects. A brief on problem: Lets take example problem for simplicity: - I have a G…
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…
The goal of the video will be to teach the user the difference and consequence of passing data by value vs passing data by reference in C++. An example of passing data by value as well as an example of passing data by reference will be be given. Bot…

807 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question