optimization, file reading and general screen questions in C

[chocsplease]

Huge thanks for all the help once again.

Originally Posted by paraj

There might be more problems, but you're writing one byte past the end of "PrefsFile":

                PrefsFile = malloc(size);
        }
        printf("OK1\n");
        fileread = Read(fh,(APTR)PrefsFile,(long)size);
        (char)PrefsFile[size]='\0'; /*put null at end of file*/ <---- Writing past the end of the allocated data

Oops - how many times have I forgotten that you count from zero not one! Ugh... Thanks for spotting this!

Originally Posted by alkis

It's the buffer overflow as paraj said. (probably)

A few other things
close(fh) should be inside the if (fh) {}

Yep, another goof I guess it would crash if the file failed to open (?)

Originally Posted by alkis

Try struct FileInfoBlock __aligned fib and then pass the address of it &fib (one thing less to malloc/free)

Could you give me a short code example? I had a lot of problems getting the fib code to work at all and pinched the code I'm using from something I found on the net.

Originally Posted by meynaf

You should write Close(fh), not close(fh).

<sigh> (there should be an Emogi for this the number of times I use it) Printf/printf Close/close... I thought that I'd left case sensitivity behind with Linux (or is it linux?)

Many thanks for spotting this. And thanks again for all the help.

[alkis]

Originally Posted by chocsplease

Could you give me a short code example? I had a lot of problems getting the fib code to work at all and pinched the code I'm using from something I found on the net.

Sure, have a look at this.

#include <proto/dos.h>

int main(int argc, char *argv[]) {
  struct FileInfoBlock __aligned fib;
  BPTR f;

  if (f = Open("c:list", MODE_OLDFILE)) {
    if (ExamineFH(f, &fib)) {
      Printf("Size of c:list is %ld\n", fib.fib_Size);
    }
    Close(f);
  }
  return 0;
}

Tested with

m68k-amigaos-gcc -noixemul -o fib fib.c

__aligned is a way to make the variable LONG aligned. Almost everything in AmigaDOS must be long-word aligned due to it's BCPL inheritance. The absolute correct way to get a FileInfoBlock would be to use AllocDosObject/FreeDosObject (http://amigadev.elowar.com/read/ADCD.../node013E.html), but it's way overkill for my taste for FileInfoBlock.

[bubbob42]

Originally Posted by alkis

__aligned is a way to make the variable LONG aligned. Almost everything in AmigaDOS must be long-word aligned due to it's BCPL inheritance.

Uh...until I read your post, I was somehow expecting the compiler (SAS/C in my case) to deal with the alignment issue automagically. Off to fix some possible bugs

@Thread: I'm also a big fan of ReadArgs() - it's simply Amiga style.

[Thorham]

Originally Posted by bubbob42

@Thread: I'm also a big fan of ReadArgs() - it's simply Amiga style.

While it's useful, it's still not as good as a custom function can be, AOS style or not (don't care much about that other than not needing huge hacks like ixemul, and making sure things play nice with the OS).

[meynaf]

Originally Posted by Thorham

While it's useful, it's still not as good as a custom function can be, AOS style or not (don't care much about that other than not needing huge hacks like ixemul, and making sure things play nice with the OS).

Not as good as a custom fonction ? There aren't many cases where it can't do the job. Often you need to perform extra checks of course, but it does most of the work. Do you have examples where ReadArgs isn't good enough ?

[bubbob42]

My only problem with ReadArgs() so far has been the fact it's a Kick2.x+-function (well, there is some unofficial method to use it with 1.3+) and sometimes support for 1.x is mandatory, resulting in a special 1.x binary with stripped down arguments.

[Thorham]

Originally Posted by meynaf

Do you have examples where ReadArgs isn't good enough ?

I rather get some more extensive help text when I use ? as a parameter. It's just the way it feels, however, and the function is actually fine as is.

[meynaf]

Originally Posted by Thorham

I rather get some more extensive help text when I use ? as a parameter. It's just the way it feels, however, and the function is actually fine as is.

I display extensive help when parameters are found to be incorrect. Sounds to me like the best thing to do.

[chocsplease]

Hi Again,

The ReadArgs vs Argv/Argc discussion is interesting and if I was not trying to get my executable down in size I would go the Argv/c route however I suspect (please correct if I am wrong) that one of the reasons it is so big is due to my using stdio. (?)

Anyway I'm back with another stupid problem. This is in the code I am using to allocate the messages read in the file opened in my previous posts (the closing rather than Closing of which caused the guru) to an array of pointers so that I can then display them in the main output.

My 1st effort was this line

if (mess1=="msg_readwrite") {messages[0]=mess2;err=0;}    /*change the read write message to that of prefs file*/

messages[] is defined as a global thus -

static char *messages[50]={MSG_READWRITE,MSG_NODISK,MSG_BADDISK,MSG_KICKSTART,MSG_NOT_DOS,MSG_READONLY,MSG_VALIDATING,\
    MSG_MOUNTED,MSG_VOL_HEAD,"Unit","Size","Used","Free","Errors","Status","Name",MSG_ERROR_MEM,MSG_BREAK,\
    MSG_ERROR_LIB};

and I suspect might be part of my problem...

The top of the function is as follows and it just contains a series of such if statements..

/*function to localise messages*/

int update_message(char mess1[], char mess2[])
{
    int err=1;
    int sizedef=-1;

And the function is called from readprefs, which I posted previously.

This compiles fine, but results in all the messages being the very last message in the prefs file, which is currently msg_size_t=Tb, so everything in the array messages =Tb.

However what I want is for each element in messages to point to the memory location of the new message, passed as mess2, which I malloc'd in readprefs in effect changing the message to the new one.

I've tried various combinations of

if (mess1=="msg_readwrite") {&messages[0]=&mess2;err=0;}    /*change the read write message to that of prefs file*/

and

if (mess1=="msg_readwrite") {messages[0]=&mess2;err=0;}    /*change the read write message to that of prefs file*/

but just get compile errors.

I must be getting confused about how pointers work, but can't find out quite work out where I am going wrong.

[idrougge]

You can't use == to compare strings in C. You need to use strcmp() for that.

[ajk]

Just to explain in more detail - when you do use == to compare strings, you are actually comparing the values of the pointers (memory addresses). This is a perfectly valid thing to do, but not useful towards your goal here.

[Thorham]

Originally Posted by meynaf

I display extensive help when parameters are found to be incorrect. Sounds to me like the best thing to do.

That's a good approach. Anyway, the whole readargs thing is just a feeling. I don't have rational reasons not to use it other than argc/argv being the easiest in C.

Originally Posted by chocsplease

I suspect (please correct if I am wrong) that one of the reasons it is so big is due to my using stdio. (?)

Yes. The moment you start using C runtime functions your executable will grow. It can also depend on your compiler.

Originally Posted by ajk

Just to explain in more detail - when you do use == to compare strings, you are actually comparing the values of the pointers (memory addresses). This is a perfectly valid thing to do, but not useful towards your goal here.

Indeed, you'd need a hash table to use pointer based string comparisons.

[nogginthenog]

Originally Posted by Thorham

That's a good approach. Anyway, the whole readargs thing is just a feeling. I don't have rational reasons not to use it other than argc/argv being the easiest in C.

ReadArgs() can also be used to parse files. I believe RoadShow uses it for the config files (at least they use the same syntax). I like this!

[idrougge]

Since programmers hate handling user input and writing help texts, ReadArgs is a great mediator between the programmer and user.

[chocsplease]

Hi again,

I have run into an odd problem which is affecting my code - and the original info command.

To hit it you have to do the following.

Have crossdos active on DF0: (so you have PC0:)

Put an Amiga formatted floppy disk in df0:

Format this as a PC disk using PC0:

without removing the disk or anything else, open a shell and type info - you'll get something like this -

Mounted disks:
Unit      Size      Used      Free Full Errs   Status   Name
SMBFS0:   280M494895104529088512   2%   0  Read/Write PCbackup
PC0:      720K      22    1418   2%   0  Read/Write EMPTY
CF0:      No disk present
RAM:      612K     612       0 100%   0  Read/Write Ram Disk
DH0:       94M  188030    5297  97%   0  Read/Write WB
DF0:      837K       2    1756   0%   0  Read/Write Empty
CC0:      No disk present
DF1:      No disk present
DH1:    -639321K 6867197  242769  97%   0  Read/Write Games
DH2:    -540681K 7142379  164868  98%   0  Read/Write Apps

Volumes available:
EMPTY [Mounted]
Empty [Mounted]
PCbackup [Mounted]
Ram Disk [Mounted]
Apps [Mounted]
Games [Mounted]
WB [Mounted]

Both PC0: and DF0: are reporting disks, which is wrong. The DF0: should say it cant read it.

I think the problem is that the format command is not refreshing the dos info once it is completed, or something else is not refreshing it.

If you remove and re-insert the floppy then the system refreshes as you would expect and info reports correctly.

Can anyone think of a way around this? Is there a way of forcing a dos refresh?

Thanks in advance.

[alkis]

Untested.

Try

Inhibit("df0:",TRUE);
Inhibit("df0:",FALSE);
Inhibit("pc0:",TRUE);
Inhibit("pc0:",FALSE);

http://amigadev.elowar.com/read/ADCD.../node017E.html

[idrougge]

Diskchange df0:

[chocsplease]

Thanks to all the great help folks have given I am nearing the end of my 'quest' to re-write the info command. I still have a looong way to go to be able to programme effectively in C but am getting there.

Anyway thanks to Thorham's post on the 1st page I now have a list of the reddest, bluest and greenest colours available on the users workbench screen. But (you could feel that coming I guess) I have hit another hurdle in that I can't for the life of me work out how to change the text colour in the shell window to equal them.

It looks like the shell uses escape codes and also that it is limited to the number of colours it can use, is this the case?

If so is there any way to get it to change the text to say colour 15 on a 32 colour screen?

I read about the Write command here - http://amigadev.elowar.com/read/ADCD.../node0102.html

And didn't understand it at all - other than the escape codes bit.

Could someone point me at a nice - simple - example of changing the text colours in the shell window to one available on the workbench screen?

I hope this is possible, as always many thanks in advance.