Let's get better in C - Exercise #2 - What's the problem with the following code?

+5 votes
asked Sep 17 by Peter Minarik (12,220 points)
edited Sep 21 by Peter Minarik

Instructions

This exercise is for anyone who would like to get better in C/C++ coding.

I've provided some sample code, that has some problem with it. (At least one, that I wanted to focus on in this exercise, but accidentally I could have made multiple mistakes. XD)

Let's see who finds the problem.

Please, tell me

  1. what part of the code has a problem
  2. why it is a problem
  3. how would you fix it

The Code (with a problem):

#include <stdio.h>

void printNumber(const int * pNumber)
{
    printf("%d\n", *pNumber);
}

int main()
{
    short int number = 123;
    printNumber((int*)&number);
    return 0;
}

Update

We have a solution! See xDELLx's answer below. Thanks for finding the problem in the code.

Also, thank you for everyone for participating in this little challenge.

Good job! :)

5 Answers

+1 vote
answered Sep 19 by xDELLx (4,800 points)
selected Sep 21 by Peter Minarik
 
Best answer

Problem:

    printNumber((int*)&number)

Why its a problem :

Data type short int occupies 2 bytes in memory & int will occupy 4 bytes (generally).

Main Point to keep in mind is sizeof(short int) < sizeof(int).

Now, if we typecast the address of number (ie short data type ) & promote it to an int, we are adding additional 2 bytes of data & passing it to printNumber() function,which is wrong and will lead to corrupt data being passed to function.
Since we are working on stack memory, this will just print garbage .But when working with heap memory, this could have crashed the program, in worst case.

How to fix:

  • I would say, don't use pointers, since the data passed osnt huge , pass by value semantics would just work ok.
  • If pointers are needed, and source data(ie number variable) has to be a short & target data (argument passed to function ) will be int always, use a temp int variable which is first initialised to number & pass it as an argument to function.
  • Last , option would be typecast the *pNumber back to short int,before printing, in printNumber. Although the above 2 options are safer as no unwanted data is passed around. ie  /*on Line # 5 */ printf(" %d\n", (short int)*pNumber);

Hope below code helps ✌.


#include <stdio.h>

void printNumber(const int * pNumber)
{
    printf(" %d\n", *pNumber);
}

int main()
{
    short int number = 123;
    short int x = 456;      // dummy variable to indicate that other variables
                            //would be part of larger code
    int temp =0;                            
    printNumber((int*)&number);
    //fix below
    temp = number;
    printNumber(&temp);
    
    printf("sizeof > SHORT %ld :: INT %ld\n", sizeof(short int), sizeof(int) );
    return 0;
}
/*

Output below:
 29884539
 123
sizeof > SHORT 2 :: INT 4

*/

commented Sep 21 by Peter Minarik (12,220 points)
It seems like we have our winner for this challenge.

Yes, indeed the problem is that our printNumber function expects an int pointer (size of 4 bytes), but out data (number) is stored on short int (size of 2 bytes). When you cast a pointer to a 2-byte data to a pointer of 4-byte data, you will take 2 additional bytes from the memory's surrounding area practically adding garbage value to your original value (number).

So the solution is to never do this. Do not cast a pointer of a certain size to a pointer of a different size. Use a temporary variable, if necessary.

short int number = 123;
int intNumber = number;
printNumber(&intNumber);

Well done, xDELLx.
0 votes
answered Sep 17 by yousef shsaheen (140 points)
;(void printNumber(const int * pNumber)
0 votes
answered Sep 18 by Joseph Judge (140 points)
The call to the function is casting as an integer pointer the address of the variable and the declaration line of the function is using a constant which is going to serve a limited use, like one time.
0 votes
answered Sep 19 by Sawapandeep Singh (140 points)
i just checked your code its alright no error is detected . MAYBE! you havent selected the language in the top right corner of the page { onlinegdb.com }
commented Sep 19 by Admin (4,350 points)
Code is compilable and executing.
But still there is/are hidden logical and programming mistake(s).
0 votes
answered Sep 19 by Admin (4,350 points)
I noticed there is one classic mistake handling pointers.
However, I am not sure if constant qualifier can lead to some other mistake or not.
Welcome to OnlineGDB Q&A, where you can ask questions related to programming and OnlineGDB IDE and and receive answers from other members of the community.
...