pointer, c++

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!

rbhargawFounderAsked:
Who is Participating?
 
evilrixSenior Software Engineer (Avast)Commented:
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
 
ZoppoCommented:
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
 
KelarosCommented:
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
Cloud Class® Course: Ruby Fundamentals

This course will introduce you to Ruby, as well as teach you about classes, methods, variables, data structures, loops, enumerable methods, and finishing touches.

 
evilrixSenior Software Engineer (Avast)Commented:
>> 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
 
rbhargawFounderAuthor Commented:
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
 
stachenovCommented:
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
 
rbhargawFounderAuthor Commented:
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
 
rbhargawFounderAuthor Commented:
short nValue1 = 1;

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

Trying this but getting heap error but it ran once!
0
 
stachenovCommented:
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
 
sarabandeCommented:
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
 
evilrixSenior Software Engineer (Avast)Commented:
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
 
rbhargawFounderAuthor Commented:
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
 
stachenovCommented:
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
 
evilrixSenior Software Engineer (Avast)Commented:
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
 
sarabandeCommented:
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
 
stachenovCommented:
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
 
evilrixSenior Software Engineer (Avast)Commented:
As far as the question goes the following posts answer it.

http:#a36949926
http:#a36950053
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.