This is a function to return a linked list consisting of sum of two linked list. For example, Linked list 1 : 1, 2, 3 Linked list 2 :4, 9, 6 Output: 5, 1, 0, 1
struct ListNode {
int val;
struct ListNode *next;
};
struct ListNode* addTwoNumbers(struct ListNode* l1, struct ListNode* l2){
struct ListNode *head;
struct ListNode *temp = head;
int sum = 0;
int carry = 0;
while (l1 || l2 || carry) {
temp->next = malloc(sizeof(struct ListNode));
int x = (l1->val ? l1->val : 0);
int y = (l2->val ? l2->val : 0);
sum = (x y carry) % 10;
carry = (x y carry) / 10;
temp->next->val = sum;
l1 = (l1 ? l1->next : NULL);
l2 = (l2 ? l2->next : NULL);
temp->next->next = NULL;
temp = temp->next;
}
return (head->next);
}
I don't know what causes an error.
When I declare int sum as
int sum = (l1 != NULL ? l1->val : 0) (l2 != NULL ? l2->val : 0) carry;
There's no problem. However, If I try a different way
if (l1 != NULL) x = l1->val;
if (l2 != NULL) y = l2->val;
int sum = x y carry;
It causes time limit exceeded error in some cases. Only thing I can think of is that int data type might not be enough to store some numbers. However the input that causes time limit exceeded is simply [9,9,9,9,9,9,9,9] and [9,9,9,9] linked list which is just 9 9 case. So, I'm lost.
CodePudding user response:
Two issues:
head
is never initialised, so that leads to undefined behaviour. As it seems you want to have a dummy node for it (as a pre-head node), you should initialise with a newly allocated node.The expressions for
x
andy
potentially dereference a NULL pointer. Instead of checkingl1->val
you should be checkingl1
.
struct ListNode* addTwoNumbers(struct ListNode* l1, struct ListNode* l2){
struct ListNode *head malloc(sizeof(struct ListNode)); // Corrected
struct ListNode *temp = head;
int sum = 0;
int carry = 0;
while (l1 || l2 || carry) {
temp->next = malloc(sizeof(struct ListNode));
int x = l1 ? l1->val : 0; // Corrected
int y = l2 ? l2->val : 0; // Corrected
sum = (x y carry) % 10;
carry = (x y carry) / 10;
temp->next->val = sum;
l1 = l1 ? l1->next : NULL;
l2 = l2 ? l2->next : NULL;
temp->next->next = NULL;
temp = temp->next;
}
return head->next;
}
You could then restructure the code a bit to:
- avoid code repetition;
- use more relevant variable names;
- be explicit with
NULL
comparisons
struct ListNode* Node(int value) {
struct ListNode *newNode = malloc(sizeof(struct ListNode));
newNode->val = value;
newNode->next = NULL;
return newNode;
}
struct ListNode* addTwoNumbers(struct ListNode *l1, struct ListNode *l2) {
struct ListNode *preHead = Node(0);
struct ListNode *tail = preHead;
int carry = 0;
while (l1 != NULL || l2 != NULL || carry) {
int sum = (l1 != NULL ? l1->val : 0) (l2 != NULL ? l2->val : 0) carry;
tail = tail->next = Node(sum % 10);
carry = sum / 10;
if (l1 != NULL) l1 = l1->next;
if (l2 != NULL) l2 = l2->next;
}
return preHead->next;
}