From:	eglebbk AT dds DOT nl (Evert Glebbeek)
Newsgroups:	comp.os.msdos.djgpp
Subject:	Re: having problem with malloc
Date:	Mon, 31 Jul 2000 00:06:28 GMT
Organization:	Physics student, University of Amsterdam
Lines:	126
Distribution:	world
Message-ID:	<3984b5d5.48960018@news.wins.uva.nl>
References:	<39847485 DOT 5822 AT virgin DOT net>
NNTP-Posting-Host:	stol-117-205.uva.studentennet.nl
X-Trace:	info.wins.uva.nl 965001869 14684 145.98.117.205 (31 Jul 2000 00:04:29 GMT)
X-Complaints-To:	usenet AT science DOT uva DOT nl
NNTP-Posting-Date:	Mon, 31 Jul 2000 00:04:29 +0000 (UTC)
X-Newsreader:	Forte Free Agent 1.11/32.235
To:	djgpp AT delorie DOT com
DJ-Gateway:	from newsgroup comp.os.msdos.djgpp
Reply-To:	djgpp AT delorie DOT com

Newsgroup: comp.os.msdos.djgpp
 From: Steven salt <steven DOT salt AT virgin DOT net>
 On Sun, 30 Jul 2000 19:31:33 +0100

>I have attached code 
better to quote the code in your mail. IIRC, not all newsreaders
handle attachments properly.
>but I don't seem to be able to do any more
>mallocs!!
Could it be that you've run out of memory? Seems unlikely for a simple
program though. What are the errors you get? Where does your program
abort? How did you compile it?
>my program is well commented!!
It contains a lot of comments, but IMHO, they are not always
enlightening as to what your code is supposed to do.
It's also a bit long to work through. Anyway, I'll make any remarks I
can.
>#define end_of_line 10
Not an error, but there is acustom to write #defines in all capitals,
and variables/functions in all lowercase to make it easier to
distinguish them.
>typedef struct {
>
>} String;
What's this supposed to do?
>typedef struct {
>unsigned char file[25];
>unsigned char *filedump;
>int fileLen;
>int lineNum;
>int *lines;
>int EndOfFile;
>int scanNum;
>String scanCode[10];
>} scan;
It's considered bad coding by some to do mixed case names. Instead,
the suggestion is to use underscores, eg, end_of_file.
This took me some getting used to, but now that I am used to it, I
find it much easier to read.
Also a bit of personal advise: I find it useful to include a comment
at the end of each entry in a struct to describe what it's supposed to
be, regardless of how meaningful the name is. Then again, I've been
known to overcomment.

>/* create allocate memory for filedump and fill with NULL charictors*/
>info.filedump=(char *)malloc(info.fileLen + 1); 
>memset(info.filedump, NULL, info.fileLen + 1); 
I think you mean `0' rather than `NULL'

>(int *)info.EndOfFile = info.filedump + info.fileLen;
This is going to do Something Bad to your program if I'm not mistaken.
inf.EndOfFile is an integer, which you cast to a pointer to an integer
(probably an invalid pointer) and then store something to.
>/* ##################### FUNCTIONS!!  ######################## */
>
>int getfilelen(unsigned char *file) {
There has to be a library function to do this (or maybe it's stored in
the FILE * structure); though it's not standard ANSI. Might be Posix,
but someone else would probably know better.
>int index;
>FILE *fp;
>char c;
>index = 0;
>fp = fopen(file,"r");
I suggest opening the file in binary mode: "rb"
>if (fp == NULL) printf("File doesn't exist\n");
I think you should abort your program in case of an error, or return
an error code (such as -1 for the length of a file).
Also, if statements are often easier to read if you put the actions on
a seperate line from the conditions.
>else {
>do {
>c = getc(fp);    /* get one character from the file */
>
>index++;
>} while (c != EOF);    /* repeat until EOF (end of file)  */
I think you could write
while (getc(fp)!=EOF)
   index++;
>}
>fclose(fp);
>return (index);
>}
>
>void fillbuf(unsigned char *file, unsigned char *buf) {
see comments above

[snip]
>void getlinepointers(unsigned char *buf, int *linepointers, int len) {
>int x;
>int index;
>linepointers[0] = 0;
linepointers[0] = NULL;
>index = 1;
What's the following piece of code supposed to do? I'm sorry, but I
don't understand it.
>for (x = 0; x < len; x++) {
>if (buf[x] == end_of_line) {
>/* base of memory address + location of line feed char + 1 to get 
>location to get location of begging of line */  
>(int *)linepointers[index] = buf + x + 1;
Again, you do a cast to a pointer where you don't want it.
>index++;
>}
>
>}
>} 
>
>
>void textalloc(unsigned char *buf, int size) {
>buf = (unsigned char *)malloc(size + 1);
>memset(buf, NULL, size + 1);
Again, I think you meant 0 istead of NULL. If you want to allocate a
block of memory set to all 0's, as it looks like you want, the library
function calloc does that, so you could use that rather than malloc.
>}

Anyway, there seem to me to be quite a lot of errors in your program.
Did it really compile properly?
Also, one last general hint of advise: indent your code. It's much
easier to read and maintain because you can see the logical structure
of the code at a glance (well, more or less).

HTH,

Evert Glebbeek

- Raw text -