0a appending w/strcpy

[Grant Austin]

I have a linked list where each node holds a couple numbers a char
and a const char array. I use strcpy to take an char array
argument passed to my insert function to copy the string into
the new node.

To some of these entries 0a(line feed?) is being appended.
I've looked through the faq and googled but haven't found much
info that applies.

The code follows this post(in a bit I'll split it into multiple
files for use in a larger project).

Regards,
Newbie Grant

<-------------------------------

#include <stdio.h>
#include <stdlib.h>
struct motNode {
struct motNode * next;
unsigned int opCode;
char type;
unsigned int numArgs;
char opName[];
};
typedef struct motNode motList;
motList *mot_head, * mot_cur;
int mot_insert(unsi gned int opCode, char type, unsigned int numArgs,
const char * opName){
mot_cur = (motList *)malloc(sizeof (motList));
mot_cur->opCode = opCode;
mot_cur->type = type;
mot_cur->numArgs= numArgs;
strcpy(mot_cur->opName, opName);
mot_cur->next = mot_head;
mot_head = mot_cur;
}

mot_dump(){
while(mot_cur){
printf("%s\n",m ot_cur->opName);
mot_cur = mot_cur->next;
}
}

generate_mot(){
mot_head = NULL;
mot_insert(0,'r ',0,"hlt");
mot_insert(1,'r ',3,"add");
mot_insert(2,'r ',3,"sub");
mot_insert(3,'r ',3,"mul");
mot_insert(4,'r ',3,"div");
mot_insert(5,'r ',3,"mod");
mot_insert(6,'r ',2,"move");
mot_insert(7,'r ',3,"and");
mot_insert(8,'r ',3,"or");
mot_insert(9,'r ',3,"xor");
mot_insert(10,' r',2,"com");
mot_insert(11,' r',3,"sll");
mot_insert(12,' r',3,"srl");
mot_insert(13,' r',3,"sra");
mot_insert(14,' r',3,"jr");
mot_insert(15,' r',1,"rdr");
mot_insert(16,' r',1,"prr");
mot_insert(17,' r',1,"prh");

/* I format */
mot_insert(18,' i',2,"li");
mot_insert(19,' i',3,"addi");
mot_insert(20,' i',3,"subi");
mot_insert(21,' i',3,"muli");
mot_insert(22,' i',3,"divi");
mot_insert(23,' i',3,"modi");
mot_insert(24,' i',3,"lwb");
mot_insert(25,' i',3,"swb");

/* J format */
mot_insert(26,' j',2,"lwa");
mot_insert(27,' i',2,"swa");
mot_insert(28,' i',1,"j");
mot_insert(29,' i',1,"jal");
mot_insert(30,' i',1,"jeq");
mot_insert(31,' i',1,"jne");
mot_insert(32,' i',1,"jlt");
mot_insert(33,' i',1,"jle");
mot_insert(34,' i',1,"jgt");
mot_insert(35,' i',1,"jge");
}

main(int argv, char * argc[]){
FILE * infle;
int
eofl,
fcount = 2;

if(argv < 3){
fprintf(stderr, "Too few arguments\n");
fprintf(stderr, "\tusage:\t p2 <flag> <input files:minimum of one>\n");
exit(1);
}
generate_mot();
mot_dump();
do{
printf("infle :: %s\n",argc[fcount]);
if((infle = fopen(argc[fcount],"r")) == NULL){
fprintf(stderr, "Unable to open data file %s\n",argc[fcount]);
}
else{
//do work
fclose(infle);
}
fcount++;
} while(fcount < argv);

}

--------------------------------->

[Peter Pichler]

"Grant Austin" <ga*****@foo.fo o.bar.net> wrote in message
news:pa******** *************** *****@foo.foo.b ar.net...

I have a linked list where each node holds a couple numbers a char
and a const char array. I use strcpy to take an char array
argument passed to my insert function to copy the string into
the new node.

To some of these entries 0a(line feed?) is being appended.
I've looked through the faq and googled but haven't found much
info that applies.

The code follows this post(in a bit I'll split it into multiple
files for use in a larger project).

Regards,
Newbie Grant

<-------------------------------

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

struct motNode {
struct motNode * next;
unsigned int opCode;
char type;
unsigned int numArgs;
char opName[];
};

typedef struct motNode motList;
The two declarations above could be combined into one, did you know it?
Mind you, I wouldn't do it, typedefing structures is rarely a good idea.

But the most important point and probably a source of your problem is
the declaration of your opName field. You really need to give it a size.
I am surprised it even compiles.
motList *mot_head, * mot_cur;
Globals are another thing that is almost never a good idea.
int mot_insert(unsi gned int opCode, char type, unsigned int numArgs,
const char * opName){
mot_cur = (motList *)malloc(sizeof (motList));
Two things here. First, do not cast the return value from maloc(). It is
not necessary in C and may hide potential problems, when you forget to
include stdlib.h. Second, you need to check the malloc()'s return value.
mot_cur->opCode = opCode;
mot_cur->type = type;
mot_cur->numArgs= numArgs;
strcpy(mot_cur->opName, opName);
Here is your problem. You copy something to opName, but how much room
you have in opName? A typical example of UB (Undefined Behaviour).
mot_cur->next = mot_head;
mot_head = mot_cur;
}
Your function is declared as returning int, so where is it? Better yet,
let it return a pointer to your new created node and get rid of a global
variable.
mot_dump(){
while(mot_cur){
printf("%s\n",m ot_cur->opName);
mot_cur = mot_cur->next;
}
} Another function that fails to return int, even though it is declared
as such (although implicitly). Also, it relies on mot_cur being set up
before calling. Why don't you set it up in the function? Then you could
get rid of another global variable. It would need a pointer to a head
as a parameter.
generate_mot(){
mot_head = NULL;
mot_insert(0,'r ',0,"hlt");
mot_insert(1,'r ',3,"add"); .... mot_insert(34,' i',1,"jgt");
mot_insert(35,' i',1,"jge");
}
Yet again, where's your return?
main(int argv, char * argc[]){
FILE * infle;
int
eofl,
fcount = 2;

if(argv < 3){
fprintf(stderr, "Too few arguments\n");
fprintf(stderr, "\tusage:\t p2 <flag> <input files:minimum of one>\n");
exit(1);
1 is not a portable value to be returned from main() or used in exit().
Use EXIT_SUCCESS or EXIT_FAILURE, both defined in stdlib.h, which you
already include.
}
generate_mot();
mot_dump();
do{
printf("infle :: %s\n",argc[fcount]);
if((infle = fopen(argc[fcount],"r")) == NULL){
fprintf(stderr, "Unable to open data file %s\n",argc[fcount]);
}
else{
//do work
fclose(infle);
}
fcount++;
} while(fcount < argv);
return EXIT_SUCCESS;
}

--------------------------------->

[Grant Austin]

Thanks for all of the comments, things are becoming clearer.
I'll be digesting for a while. (=

I'm creating the list as global because I will need access to it
in functions in other files. I thought that there might be quite
a bit of overhead passing the list to those functions. Now I realize
I can just pass a pointer to the head of the list.

Regards,
-G

[Al Bowers]

Grant Austin wrote:

I have a linked list where each node holds a couple numbers a char
and a const char array. I use strcpy to take an char array
argument passed to my insert function to copy the string into
the new node.

To some of these entries 0a(line feed?) is being appended.
I've looked through the faq and googled but haven't found much
info that applies.

.............co de snipped.......
There are many errors.
The major error is that you did not allocate storage space for the
struct that will store the string. Here is your code modified to
show one way you might want to define function mot_insert.

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

struct motNode
{
struct motNode * next;
unsigned int opCode;
char type;
unsigned int numArgs;
char *opName;
};
typedef struct motNode mot_List;

int mot_insert(mot_ List **mot_head,unsi gned int opCode, char type,
unsigned int numArgs, const char * opName);
void mot_dump(mot_Li st *mot_cur);
void generate_mot(mo t_List **mot_cur);
void mot_free(mot_Li st **mot_cur);

int main(void)
{
mot_List *mot_head = NULL;

generate_mot(&m ot_head);
mot_dump(mot_he ad);
mot_free(&mot_h ead);
return 0;
}

int mot_insert(mot_ List **mot_head,unsi gned int opCode, char type,
unsigned int numArgs, const char * opName)
{
mot_List *mot_cur = malloc(sizeof *mot_cur);

if(mot_cur == NULL) return 0;
if((mot_cur->opName = malloc(strlen(o pName)+1)) == NULL)
{
free(mot_cur);
return 0;
}
mot_cur->opCode = opCode;
mot_cur->type = type;
mot_cur->numArgs= numArgs;
strcpy(mot_cur->opName, opName);
mot_cur->next = *mot_head;
*mot_head = mot_cur;
return 1;
}

void mot_dump(mot_Li st *mot_cur)
{
while(mot_cur)
{
printf("%s\n",m ot_cur->opName);
mot_cur = mot_cur->next;
}
}

void generate_mot(mo t_List **mot_cur)
{
mot_insert(mot_ cur,0,'r',0,"hl t");
mot_insert(mot_ cur,1,'r',3,"ad d");
mot_insert(mot_ cur,2,'r',3,"su b");
mot_insert(mot_ cur,3,'r',3,"mu l");
mot_insert(mot_ cur,4,'r',3,"di v");
mot_insert(mot_ cur,5,'r',3,"mo d");
mot_insert(mot_ cur,6,'r',2,"mo ve");
mot_insert(mot_ cur,7,'r',3,"an d");
mot_insert(mot_ cur,8,'r',3,"or ");
mot_insert(mot_ cur,9,'r',3,"xo r");
mot_insert(mot_ cur,10,'r',2,"c om");
mot_insert(mot_ cur,11,'r',3,"s ll");
mot_insert(mot_ cur,12,'r',3,"s rl");
mot_insert(mot_ cur,13,'r',3,"s ra");
mot_insert(mot_ cur,14,'r',3,"j r");
mot_insert(mot_ cur,15,'r',1,"r dr");
mot_insert(mot_ cur,16,'r',1,"p rr");
mot_insert(mot_ cur,17,'r',1,"p rh");

/* I format */
mot_insert(mot_ cur,18,'i',2,"l i");
mot_insert(mot_ cur,19,'i',3,"a ddi");
mot_insert(mot_ cur,20,'i',3,"s ubi");
mot_insert(mot_ cur,21,'i',3,"m uli");
mot_insert(mot_ cur,22,'i',3,"d ivi");
mot_insert(mot_ cur,23,'i',3,"m odi");
mot_insert(mot_ cur,24,'i',3,"l wb");
mot_insert(mot_ cur,25,'i',3,"s wb");

/* J format */
mot_insert(mot_ cur,26,'j',2,"l wa");
mot_insert(mot_ cur,27,'i',2,"s wa");
mot_insert(mot_ cur,28,'i',1,"j ");
mot_insert(mot_ cur,29,'i',1,"j al");
mot_insert(mot_ cur,30,'i',1,"j eq");
mot_insert(mot_ cur,31,'i',1,"j ne");
mot_insert(mot_ cur,32,'i',1,"j lt");
mot_insert(mot_ cur,33,'i',1,"j le");
mot_insert(mot_ cur,34,'i',1,"j gt");
mot_insert(mot_ cur,35,'i',1,"j ge");
}

void mot_free(mot_Li st **mot_cur)
{
mot_List *tmp;

for( ; *mot_cur; *mot_cur = tmp)
{
tmp = (*mot_cur)->next;
free(*mot_cur);
}
return;
}
--
Al Bowers
Tampa, Fl USA
mailto: xa******@myrapi dsys.com (remove the x to send email)
http://www.geocities.com/abowers822/

[Grant Austin]

On Sat, 28 Feb 2004 12:17:34 -0500, Al Bowers wrote:
....text snipped...

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

struct motNode
{
struct motNode * next;
unsigned int opCode;
char type;
unsigned int numArgs;
char *opName;
};
typedef struct motNode mot_List;

int mot_insert(mot_ List **mot_head,unsi gned int opCode, char type,
unsigned int numArgs, const char * opName);
void mot_dump(mot_Li st *mot_cur);
void generate_mot(mo t_List **mot_cur);
void mot_free(mot_Li st **mot_cur);

int main(void)
{
mot_List *mot_head = NULL;

generate_mot(&m ot_head);
mot_dump(mot_he ad);
mot_free(&mot_h ead);
return 0;
}

....code snipped...

Thanks for the suggestions & code. With all of the help
I'm getting here I might have a chance at not sucking in a couple
years.

I'm not sure I understand what is meant by mot_List **mot_cur
and mot_List *mot_cur. Is **mot_cur a reference to a pointer? Is *mot_cur
the value of the pointer?
Thanks,
Grant

[Chris Torek]

>"Grant Austin" <ga*****@foo.fo o.bar.net> wrote in message

news:pa******* *************** ******@foo.foo. bar.net...

struct motNode {
struct motNode * next;
unsigned int opCode;
char type;
unsigned int numArgs;
char opName[];
};

typedef struct motNode motList;
In article <news:40******* *@mk-nntp-2.news.uk.tisca li.com>
Peter Pichler <pi*****@pobox. sk> writes:

The two declarations above could be combined into one, did you know it?
Mind you, I wouldn't do it, typedefing structures is rarely a good idea.
Also, rather than combining them, if one really likes typedefs, one
should write the typedef first:

typedef struct X stX;

because that allows the alias to occur inside the structure:

struct X {
stX *next; /* linked list of struct X */
...
};

The "combo thing", "typedef struct X { ... } stX;", gives you the
worst of both worlds: not only do you have an not-strictly-necessary
alias just to avoid typing the letters "ruct" :-) (in this case);
you cannot even use it inside the braces.
But the most important point and probably a source of your problem is
the declaration of your opName field. You really need to give it a size.
I am surprised it even compiles.

This kind of declaration is a new C99-only "feature" called a
"flexible array member". The effect is as if you declared the
array with size zero (if C had zero-sized objects, which it does
not even if it should [in my opinion]).
mot_cur = (motList *)malloc(sizeof (motList));

Two things here. First, do not cast the return value from maloc(). It is
not necessary in C and may hide potential problems, when you forget to
include stdlib.h. Second, you need to check the malloc()'s return value.

And if one is going to use the new C99-only feature, one must also add
space for the name:

mot_cur->opCode = opCode;
mot_cur->type = type;
mot_cur->numArgs= numArgs;
strcpy(mot_cur->opName, opName);

Here is your problem. You copy something to opName, but how much room
you have in opName? A typical example of UB (Undefined Behaviour).

Indeed. Hence:

mot_cur = malloc(sizeof *mot_cur) + strlen(opName) + 1;

Note that a string whose strlen() is L requires L+1 bytes to store,
because one byte is needed for the '\0' marking the end.

As the FAQ notes, most (though not necessarily all) C89 compilers
allow you to use undefined behavior usefully by making the array
have size 1:

struct S {
...
char name[1];
};
...
struct S *p = malloc(sizeof *p + strlen(name));
...
strcpy(p->name, name);

In this case, the "+1" for the terminating '\0' is handled by giving
the array a size of 1. Unfortunately, the undefined behavior aspect
means you cannot count on this working on other systems, even if
it works on yours -- so you have to decide whether any savings you
find by using this so-called "struct hack" is worth the loss of
portability. (I have used it myself, in code that is not meant to
be strictly portable, in the data structures that describe potential
and ongoing DMA operations in devices. The NetBSD / FreeBSD
device-DMA model is based on this same idea, and in fact, the names
in a bus_dmamap_t are the ones I suggested. DMA areas are described
by a sequence of <ds_addr,ds_len > pairs, stored in an array allocated
via the "struct hack".)
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) [phone]
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.

[Grant Austin]

On Sat, 28 Feb 2004 16:39:43 +0000, Peter Pichler wrote:
.....snip

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

struct motNode {
struct motNode * next;
unsigned int opCode;
char type;
unsigned int numArgs;
char opName[];
};

typedef struct motNode motList;

The two declarations above could be combined into one, did you know it?

....snip...

Not that I'm going to do it but do you mean you can do something
like this?:

typedef struct motNode{
struct motNode * next;
unsigned int opCode;
..
..
} motList;
-G

[Peter Pichler]

"Grant Austin" <ga*****@foo.fo o.bar.net> wrote in message
news:pa******** *************** *****@foo.foo.b ar.net...

On Sat, 28 Feb 2004 16:39:43 +0000, Peter Pichler wrote:
....snip

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

struct motNode {
struct motNode * next;
unsigned int opCode;
char type;
unsigned int numArgs;
char opName[];
};

typedef struct motNode motList;

The two declarations above could be combined into one, did you know it?

...snip...

Not that I'm going to do it but do you mean you can do something
like this?:

typedef struct motNode{
struct motNode * next;
unsigned int opCode;
..
..
} motList;

Yes, that's what I meant. But read Chris Torek's post first, before dashing
off to do it ;-)

Peter

[Peter Pichler]

"Chris Torek" <no****@torek.n et> wrote in message

"Grant Austin" <ga*****@foo.fo o.bar.net> wrote in message

Peter Pichler <pi*****@pobox. sk> writes:

....

mot_cur = (motList *)malloc(sizeof (motList)); .... mot_cur->opCode = opCode;
mot_cur->type = type;
mot_cur->numArgs= numArgs;
strcpy(mot_cur->opName, opName);

Here is your problem. You copy something to opName, but how much room
you have in opName? A typical example of UB (Undefined Behaviour).

Indeed. Hence:

mot_cur = malloc(sizeof *mot_cur) + strlen(opName) + 1;

ITYM moving this----------------------^ here----------------^ ;-)

Thanks for the other comments.

Peter

[Grant Austin]

On Sat, 28 Feb 2004 17:57:35 +0000, Chris Torek wrote:

Indeed. Hence:

mot_cur = malloc(sizeof *mot_cur) + strlen(opName) + 1;

I'm doing something like this now, thanks to Mr. Bowers:
motList *mot_cur = malloc(sizeof *mot_cur);
mot_cur->opName = malloc(strlen(o pName)+1));

This looks like the same thing just longer.

(I do check what malloc returns in the actual code)

Thanks for the comments. My mind is trying to cave in on itself.
I'm going to go decompress before I take a crack at the hash table
where each node contains a list of integers.

If anyone was wondering and hadn't figured it out -- I'm writing
a one pass assembler.
Regards,
Grant