pointer/linked List clarification

[Peter]

The way I'm reading the following fragments is, The function call load()
passes a pointer startPtr which I believe is to be the pointer to the head
of the list " Is this correct".

Next: In the line if(previousPtr == NULL) if True the list is EMPTY "Is this
correct" so the new node "newPtr" is inserted and
newPtr->nextPtr is pointed to the head pointer "*sPtr" and the head pointer
"*sPtr" is then pointed to the new node newPtr. "Is this correct"

The head pointer points to the first node or is NULL at all times "is this
correct"

If complete code is required to understand my request say so and I'll post
the main() and load() function including the header file that contains the
structure etc.

Thank you

/* fragment from main() call to load function*/

LISTNODEPTR startPtr = NULL;

load(&startPtr, item, posn);


/* fragment form load() function*/

LISTNODEPTR load (LISTNODEPTR *sPtr, const char *value, int index)

LISTNODEPTR newPtr, previousPtr, currentPtr;


while(currentPtr != NULL && index >= currentPtr->position)
{
previousPtr = currentPtr;
currentPtr = currentPtr->nextPtr;
}
if(previousPtr == NULL)
{
newPtr->nextPtr = *sPtr;
*sPtr = newPtr;
}
else
{
previousPtr->nextPtr = newPtr;
newPtr->nextPtr = currentPtr;
}

 

[Peter]

I thought sending all this to get clarification on a few lines was a little
extreme but here it is.
Hope you can help. I don't wont to know why *LISNODEPTR just what I
mentioned in my post.

Thanks


Here is the #include "LList.h"

#include <stdio.h>#include
<stdlib.h>#include
<string.h>
#include <errno.h>
#include <limits.h>

#define MAX 30/*Linked List Structure*/

struct listNode
{
int position;
char data[MAX];
struct listNode *nextPtr;
};

typedef struct listNode LISTNODE;
typedef LISTNODE *LISTNODEPTR;

LISTNODEPTR load (LISTNODEPTR *, const char *, int);

/*------------------------main() and load()
function------------------------------*/


#include "LList.h"



int main(void)
{
LISTNODEPTR startPtr = NULL;
int choice, posn;
char item[MAX];

while(1)
{
instructions();
choice = getint();

switch (choice)
{
case 1:
printf("Enter a string: ");
fflush(stdout);
getstr(item,sizeof item);
printf("Enter position in list: ");
posn = getint();
load(&startPtr, item, posn); /* ITERESTED IN THIS LINE */
break;
case 2:
printf("Enter position to delete: ");
posn = getint();
st_priority_remv(&startPtr, posn);
break;
case 3:
printList(startPtr);
break;
}

if(choice == 4)
break;
if(choice <= 0 || choice > 4)
puts("Wrong number entered. Try again");
}

printf("End of run.\n");
freeList(&startPtr);
return 0;
}


void instructions(void) {
printf("Enter your choice:\n"
" 1 to insert a string into the list.\n"
" 2 to delete an element from the list.\n"
" 3 Print the list.\n"
" 4 to end.\n"
"Enter Option Number: ");
fflush(stdout);
}


LISTNODEPTR load (LISTNODEPTR *sPtr, const char *value, int index)
{
LISTNODEPTR newPtr, previousPtr, currentPtr;

newPtr = malloc(sizeof(LISTNODE)); if(newPtr == NULL) return NULL;




strncpy(newPtr->data, value,MAX);newPtr->data[MAX-1]
= '\0';newPtr->position
= index;newPtr->nextPtr = NULL;
previousPtr = NULL;currentPtr
= *sPtr;


while(currentPtr != NULL && index >= currentPtr->position)
{
previousPtr = currentPtr; currentPtr = currentPtr->nextPtr;
}
if(previousPtr == NULL) /* ITERESTED IN THESE 5
LINES */
{
newPtr->nextPtr = *sPtr;
sPtr = newPtr; }
else
{
previousPtr->nextPtr = newPtr; newPtr->nextPtr = currentPtr;
}
return newPtr;
}

 

[Chad]

It actually passes a pointer to startPtr. Care to share with us what
LISTNODEPTR is? I'm guessing it's something like

    typedef struct list_node* LISTNODEPTR;

Seems to just obfuscate things.

Assuming that

typedef struct list_node* LISTNODEPTR;

Then wouldn't the variable startPtr in the following function

load(&startPtr, item, posn);

be the pointer variable vs the value which strPtr points to?

 

[Barry Schwarz]

On Tue, 23 Dec 2008 16:15:26 +1100, "Peter"

Please don't top post. I have moved your second message below the
first

snip

snip

It will be once you correct the mistakes in your code.

snip

I thought sending all this to get clarification on a few lines was a little
extreme but here it is.
Hope you can help. I don't wont to know why *LISNODEPTR just what I
mentioned in my post.

Here is the #include "LList.h"

#include <stdio.h>#include
<stdlib.h>#include
<string.h>

How did you end up with such a lousy structure in your code. #include
must be the first non-white space on the line.

#include <errno.h>
#include <limits.h>

#define MAX 30/*Linked List Structure*/

struct listNode
{
int position;
char data[MAX];
struct listNode *nextPtr;
};

typedef struct listNode LISTNODE;

Normally, it is undesirable to hide the true type like this.

typedef LISTNODE *LISTNODEPTR;

Hiding pointer types like this only leads to confusion.

LISTNODEPTR load (LISTNODEPTR *, const char *, int);

/*------------------------main() and load()
function------------------------------*/


#include "LList.h"



int main(void)
{
LISTNODEPTR startPtr = NULL;
int choice, posn;
char item[MAX];

while(1)
{
instructions();
choice = getint();

switch (choice)
{
case 1:
printf("Enter a string: ");
fflush(stdout);
getstr(item,sizeof item);
printf("Enter position in list: ");
posn = getint();
load(&startPtr, item, posn); /* ITERESTED IN THIS LINE */

Already addressed.

snip

LISTNODEPTR load (LISTNODEPTR *sPtr, const char *value, int index)
{
LISTNODEPTR newPtr, previousPtr, currentPtr;

newPtr = malloc(sizeof(LISTNODE)); if(newPtr == NULL) return NULL;

Why do you think leaving this much vertical white space in your code
is a good idea?

strncpy(newPtr->data, value,MAX);newPtr->data[MAX-1]
= '\0';newPtr->position
= index;newPtr->nextPtr = NULL;
previousPtr = NULL;currentPtr
= *sPtr;

At this point, previousPtr is NULL.

while(currentPtr != NULL && index >= currentPtr->position)
{
previousPtr = currentPtr; currentPtr = currentPtr->nextPtr;
}
if(previousPtr == NULL) /* ITERESTED IN THESE 5
LINES */

This does not mean the list is empty. It means that you have reached
the end of the list and for every node in the list (possibly 0 of
them) index was >= position. It happens that the first time you call
load() the list is indeed empty but that would not be the case the
next time.

{
newPtr->nextPtr = *sPtr;
sPtr = newPtr; }

Already addressed

 

[Ben Bacarisse]

I am going to make a few remark that I don't think have been made yet.
I agree with everything else that as been said, of course.

while(1)
{
instructions();
choice = getint();
switch (choice)
{
case 1:
<snip?
}

if(choice == 4)
break;
if(choice <= 0 || choice > 4)
puts("Wrong number entered. Try again");
}

I really don't like while (1). It has the effect of starting off with
what is almost always a lie. It has its place, but not here. I'd
write:

do {
instructions();
choice = getint();
switch (choice) {
case 1:
/* action */
break;
case 2:
/* action */
break;
case 3:
/* action */
break;
case 4:
break; /* nothing to do. */
default:
/* print error message */
}
while (choice != 4);

strncpy(newPtr->data, value,MAX);

Take care here. strncpy can result is newPtr->data not being a
string. If value has MAX or more non-null characters, the result is
something that can't be used with normal string handling functions.
You can a fix this by simply adding a

newPtr->data[MAX-1] = 0;

afterwards.

 

[Peter]

By obfuscation, I mean that the name "LISTNODEPTR" itself is unpleasant
to read, suggests a macro, and is ultimately pointless since one doesn't
need a typedef to form a pointer type. Writing

struct list_node* head;

is clearer and easier to read.


It's a pointer to startPtr, not the value of startPtr.

Given

T t;

with suitable initialization,

f( &t );

passes a pointer to t, allowing the function to modify t.

f( t );

Passes a copy of t. The function cannot modify t (unless it has some
other access path).

If T is a pointer type, such as

typedef int* T;

then

f( t );

passes a pointer to int, and

f( *t );

passes a copy of the int that t points to.

Thus,

typedef struct list_node* LISTNODEPTR;
LISTNODEPTR startPtr;
...
load(&startPtr, item, posn);

passes a pointer to startPtr, which itself is a pointer to a list_node.
The load function is thus in a position to modify startPtr itself, and
also what it points to.

Exactly. However I agree with your ealier post simple is better. Thank you