0

I am doing coding with leetcode. For the question of Two add numbers, my solution could not be accepted because I didn't use new when creating struct. Here is my code:

ListNode *addTwoNumbers(ListNode *l1, ListNode *l2) {
struct ListNode temp(-1);
struct ListNode *pre = &temp;
bool PlusOne = false;
int val1 = 0;
int val2 = 0;
int sum;
while (NULL != l1 || NULL != l2)
{

    if (NULL != l1)
    {
        val1 = l1->val;
        l1 = l1->next;
    }
    if (NULL != l2)
    {
        val2 = l2->val;
        l2 = l2->next;
    }
    if (PlusOne == true)
    {
        sum = (val1 + val2 + 1) % 10;
        PlusOne = (val1 + val2 + 1) / 10;
    }
    else
    {
        sum = (val1 + val2) % 10;
        PlusOne = (val1 + val2) / 10;
    }
    struct ListNode newNode(sum);
    pre->next = &newNode;
    pre = &newNode;
    val1 = 0;
    val2 = 0;

}
if (true == PlusOne)
{
    struct ListNode newNode(1);
    pre->next = &newNode;
}
pre = temp.next;
return pre;}

It said runtime error, but it works if I use pre->next = new ListNode(1) replacing of struct ListNode newNode(1); pre->next = &newNode;

Is there anyone knows why?

Yutao Xing
  • 41
  • 5
  • 1
    Your program is ill-formed. In three places, you setup a return address of an automatic variable that is destroyed on scope-exit. Any usage of said-address, including dereference and even evaluation, invokes *undefined behavior*. That you bore witness to it "working" on an given platform is irrelevant, and a testimonial in confusing *observed* behavior with *defined* behavior. Invoking undefined behavior makes any such observation meaningless. – WhozCraig Jan 04 '15 at 08:16
  • Style advice: Don't do this: `if (true == PlusOne)`. `PlusOne` is a boolean already, and you probably wouldn't do `if(true == (true == PlusOne))` – milleniumbug Jan 04 '15 at 09:11
  • @milleniumbug, Please give more detail about this. I didn't understand what you meant. Thank you! – Yutao Xing Jan 04 '15 at 19:56
  • The if statement takes a boolean value, and executes instruction depending on if it's true or false. `PlusOne == true` is true if `PlusOne` is true, and is false if `PlusOne` is false. So it's redundant - just do `if (PlusOne)`. The same can be said about `PlusOne == false` - just do `!PlusOne`. – milleniumbug Jan 04 '15 at 20:19

1 Answers1

2

It said runtime error, but it works if I use pre->next = new ListNode(1) replacing of struct ListNode newNode(1); pre->next = &newNode;

That's because you're pointing to a local variable which will be destroyed when the if block exists.

if (true == PlusOne)
{
    struct ListNode newNode(1);
    pre->next = &newNode;
}

pre->next would be pointing to unallocated memory when the control comes out of the if block. It's similar to doing this

int* get_int() {
    int a = 1;
    return &a;
}

void process_int() {
    int *p = get_int();
    // oops! p is pointing to unallocated memory; undefined behaviour ensues
    *p;
}

Never point to a local variable which doesn't outlive your pointer.

The reason why new works is that you're manually allocating memory in the freestore which will be alive until delete is called or the program exists (whichever is earlier).

legends2k
  • 31,634
  • 25
  • 118
  • 222
  • Thank you for your quick answer! But I have tested my version using VS studio, it also worked. I understood what you are saying. So here may be something about VC++? – Yutao Xing Jan 04 '15 at 08:04
  • The last code snippet above, where I wrote as undefined behaviour, may/may not work too i.e. when you tried accessing the memory it's not been reused for something else, so it may seem that it works; but it is a time bomb. Anytime something else is written on it, your program wouldn't work. – legends2k Jan 04 '15 at 08:11
  • @YutaoXing See [here](http://stackoverflow.com/q/2397984/183120) for the reasons and to understand an important concept in C and C++. – legends2k Jan 04 '15 at 08:12