I'm trying to convert use strtok to create a linked list of strings.
I have created my own list container structure, but I don't know if I'm using strtok correctly.

uList* uS_towordlist(const char *str, const char *sep) {
	char *w, s[500];
	uList *l = uList_new();
	strcpy(s, str);
	w = strtok(s, sep);
	while (w != NULL) {
		strcpy(s, w);
		uL_pbstring(l, s);
		w = strtok(NULL, sep);
	}
	return l;
}

// heres the pbstring function
void uL_pbstring(uList *l, const char* s) {
	uListIter *i = uLI_new(E_STRING);
	pushback(l, i, (void*)s);
}

// and of course pushback

static void pushback(uList *l, uListIter *i, void *e) {
	i->val = e;
	if (l->count < 1) {
		l->first = i;
		l->last  = i;
	} else {
		l->last->next = i;
		i->prev = l->last;
		l->last = i;
	}
	l->count++;
}

I don't know if that is enough code to go off of to help me, but here is the output when I try to
make a word list:

uList *l = uS_towordlist("I like to move it move it", " ");
	uListIter *i;
	uL_foreach(i,l) {
		printf("%s\n", i->toString(i));
	}

OUTPUT

it.
it.
it.
it.
it.
it.

You do seem to be using strtok more or less correctly, however you are changing the source string
in the middle of the scan (and not using strcpy correctly at that). If you remove the strcpy of w to s
and pass w (instead of s) to uL_pbstring() it should pull off each word and put them into your list.

Also, unless that assignment of e to i->val in pushback() is overloaded and does a copy of the data
(though you say the code is C, your 2nd code block does have a line that does seem to be C++), you
are going to be pointing to memory that is out of scope, and that will probably cause problems down
the road.

I don't believe you want to be doing your strcpy() inside the while loop in towordlist().

strtok() already marches down the string, inserting NULL bytes as it goes when you pass NULL after the first call. Copying w back onto s will just result in corruption.

When you call strtok(s, sep), it returns a pointer to s and places a NUL byte in place of the first space. The next call to strtok(NULL, sep) will return a pointer to the next byte (pointing to 'l'), up to the next NUL byte which replaces the next space (yielding "like"). If you strcpy(s, w), you are taking "like" and pasting it over "I\0li", which is probably not what you want.

Try commenting out the strcpy() and change your function to call uL_pbstring(l, w) instead.

Thank you both!

I did what quix suggested and made passed uL_pbstring(l,w) instead.
I did what Sharmair said and made a copy of the passed word before storing it.

I understand what you were thinking Sharmair about my code being more C++. It is in fact
strict ANSI C. However during a few experiments I utilized function pointers to act as object methods.
The syntax printf("%s\n", i->toString(i)); is just something left behind from playing around.

Obviously it does little good since it doesn't save you any writing. Also it the indirection may be more costly.
Anyway, just thought I would explain.

Thanks for the input.

I have found an issue with using free() with datamembers in structures that
are strings allocated with strdup. Here is a quick example of what doesn't seem to work:

char *str = "hi!";
	uList *l = (uList*) malloc(sizeof(uList));
	uListIter *i = (uListIter*) malloc(sizeof(uListIter));


	l->first = i;
	i->val = strdup(str);
	free(i->val);
	i->val = NULL;
	free(i); /* here is the error, it corrupts the heap */
	free(l);

I haven't included the definitions of uList or uListIter. I didn't see the need since
I'm pretty sure that I am doing something stupid here.

The above code is in my main() function for testing, but I am having the same issue
in the uList_delete() function that frees the memory for each uListIter in the list.

Am I missusing malloc? The only think I can think of is that strdup() includes size_t + 1
for the string terminator. Could this be the issue?

PS: I am using MS Visual C++ 2010 (if that makes a difference)

It looks fine. Use a memory debugger like valgrind and you'll have a definitive error you can work against.

Yes, strdup is going to dup the entire array. Even the delimiter. Yes, free should work fine with it. As long as you are freeing an address where its memory was created with a single call of malloc() you only need a single call of free().

Check this out though:

typedef struct Poo {
	void* element;
	int o;
} Poo;

int main(void)
{
	char* str = "hey!";
	Poo *p = (Poo*) malloc(sizeof(*p));
	p->o = 3;
	free(p); /* error here!!!!!!!!!!!!!!!!!!!!! */

	scanf_s("h");
	return 0;
}

The interesting thing is, it doesn't bomb if i remove int o; from the structure.
Is this a windows issue?

I would still suggest to you that you use valgrind if you want a real answer to why the free is failing.

$ cat lol.c
#include <stdio.h>
#include <stdlib.h>

typedef struct Poo {
	void* element;
	int o;
} Poo;
 
int main(void)
{
	char* str = "hey!";
	Poo *p = (Poo*) malloc(sizeof(*p));
	p->o = 3;
	free(p); /* error here!!!!!!!!!!!!!!!!!!!!! */
 

	return 0;
}
$ clang lol.c -o lol
$ ./lol
$ #no error

No.

Seems to have been a visual studio 2010 issue actually. I have compiled the same project in Bloodshed Dev C++ and it works fine.
I am squeamish about furthering the project in ANSI C now though. Here's a new question:

Can a dll or static lib that has been written in C++ be linked to a C application? I assume that it could if the API was more written
in a C standard than C++.

I'm trying to develop a mini language to replace Mob Progs, but I want folks to be able to use it in their C muds as well as C++.

I believe so. One problem you'll run into, though, is name mangling. C name mangling is pretty simple: if you have a function "void foobar()", the mangled name is going to be "_foobar" (if I recall correctly). In C++, due to function overloading, the argument types are encoded in the name as well, so the name would be more like "?foobar@@YAXXZ". (source). To get around that, you need to write your functions in an extern "C" block, so it knows how the names should be mangled.

extern "C" {

void foobar() 
{
}

}

interesting…

There's compile options that can disable features (which you may not need) and get rid of the name mangling as well.

@johnnyStar, your code is not declaring enough memory for your structure

#include <stdio.h>
#include <stdlib.h>
 
typedef struct Poo {
	void* element;
	int o;
} Poo;
 
int main(void)
{
	char* str = "hey!";
	Poo *p = (Poo*) malloc(sizeof(*p)); /* should be Poo *p = (Poo*) malloc(sizeof(Poo)); other wise you are declaring a space of memory the size of a pointer so on 32bit systems 4 bytes 64bit 8bytes */
	p->o = 3;
	free(p); /* error here!!!!!!!!!!!!!!!!!!!!! */
 
 
	return 0;
}

in general i roll my own tokenizer most are in bastardized c/c++ so i can handle different parse modes like quotes, special tags and everything else fun

Actually, sizeof(*p) should work fine. '*p' is an expression, which sizeof processes to determine what the type of that expression would be. Since the type of p is Poo*, the type of *p is Poo. So sizeof(*p) == sizeof(Poo).

http://stackoverflow.com/questions/51265...

As a side note, don't cast malloc().

Except when you should; from the same link,



(emphasis mine)