In this function is defective since dereferencing null is not valid so i want to change code

Question

long cread(long *xp) {
  return (xp? *xp : 0);
}

It is invalid since it could attempt to read from a null address

So the solution suggested this code

long cread_alt(long *xp){
  long tem = 0;
  if(*xp > 0){
   tem = *xp;
  }
  return tem;

But I think it is also not valid since if(*xp > 0) is still defective when the xp is pointing a null address.

So I thought this code

 long cread_alt2(long *xp){
     long tem = 0;
     if(xp != NULL){
        tem = *xp;
     }
     return tem;
 }

Do I have this correct?

Your first snipped is perfectly valid. The 2nd one has undefined behaviour when `xp == nullptr`. The 3rd truncates a `long` to `int` on return. — Swordfish, Oct 12 '18 at 16:04
The original function is perfectly fine. The second one is broken. The third is OK. — Eugene Sh., Oct 12 '18 at 16:04
`return (xp? *xp : 0);` is equivalent to `if (xp != nullptr) {return *xp;}else{return 0;}` — selbie, Oct 12 '18 at 16:18
Did you mean to make the return type of `cread_alt` and `cread_alt2` an `int`? — NathanOliver, Oct 12 '18 at 21:33

NathanOliver · Answer 1 · 2018-10-12T16:41:18.277

6

long cread(long *xp) {
  return (xp? *xp : 0);
}

is valid and this is a very common technique. The expression pointer_name can be converted to a bool and will be false if pointer_name == nullptr and true if pointer_name != nullptr. That means for the above code that *xp only occurs when you do not have a null pointer.

int cread_alt2(long *xp){
    long tem = 0;
    if(xp != NULL){
        tem = *xp;
    }
    return tem;
}

Does the same thing, it is just much more verbose about it. So, the only code that is actually invalid is cread_alt, which does dereference xp without first checking if it is null or not.

edited Oct 12 '18 at 16:41

answered Oct 12 '18 at 16:05

NathanOliver

171,901
28
288
402

`cread_alt2()` also converts a `long` to an `int` on `return`. It only does the same thing as `cread()` if `int` supports at least the range of values that `long` does, which is not required by the standard. – Peter Oct 12 '18 at 21:31
@Peter It was probably a typo from the OP. I'll ask and if it is I'll update the code. – NathanOliver Oct 12 '18 at 21:32

R Sahu · Answer 2 · 2018-10-12T16:51:10.417

2

Use of

if(xp != NULL){
  tem = *xp;
}

is valid. However, in your second snippet, you have

if(*xp > 0){
  tem = *xp;
}

Perhaps you want to use both.

if ( xp != NULL && *xp > 0 )
{
   tem = *xp;
}

If you are able to use C++11 or later, it is better coding practice to use nullptr instead of NULL.

if ( xp != nullptr && *xp > 0 )
{
   tem = *xp;
}

edited Oct 12 '18 at 16:51

answered Oct 12 '18 at 16:08

R Sahu

204,454
14
159
270

0___________ · Answer 3 · 2018-10-12T17:03:31.430

0

The original function is 100% valid code and it never dereferences the NULL pointer.

I think you should read the chapter about the ternary operators from your favourite C book

edited Oct 12 '18 at 17:03

answered Oct 12 '18 at 16:11

0___________

60,014
4
34
74

1

I write about the original code not about the alternatives – 0___________ Oct 12 '18 at 17:02

In this function is defective since dereferencing null is not valid so i want to change code

3 Answers3