DaWei

I'll have a look. Incidentally, this does not initialize a pointer: "int *pointer;". It merely declares one. It's a blank page in your address book. You need to fill it in before you go looking for a hot date. int *myArray [23] declares an array of 23 elements, each of which will hold an int* if you put one there. It's a whole dam' address book, all blank.

EDIT (after importing into my compiler):
Your code doesn't compile. If you have warnings and errors disabled or throttled back, you need to turn them all on.

You should declare functions that return nothing as void, else the will be presumed to return an int, and your returns must include a value.

In C, you can't declare a variable in the middle of nowhere and assign to it with the results of a function. Some compilers may tolerate this, but don't bank on it. Put all your declarations at the top of the scope, before any code. I refer to:
This also applies to declaring the file pointer, fp, etc., after calling some functions.

Here you are passing a variable that's never been initialized. It's not an error, just a warning. Also immaterial because you are not using it. Just litter.

Now that I have an error-free compile, I'll move on into the code.

EDIT 2: How about providing a target file to work with. Short would be fine, directly in the thread would be fine.

__________________
Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code.
Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

deanosrs

#testing the ones that don't take an argument.
#nope ; this should be invalid - instruction should not be found
#nopeydopeyopelycropoly ; should also be invalid
#NOP ; should be invalid - checking case doesn't matter
#nop $4 ; invalid - shouldn't take argument
#waitbla ; just checking works for long string starting with 4 word valid instruction
#nop ; should be valid
jr bgdhfh
j
b
add
#sum
mul
nor
lw
sw

I've been testing with that. The program should ignore all lines starting with # and verify the rest of the instructions it finds.

Thanks for all this help!

DaWei

My initial output, first run (after fixing), no look at code, yet. Comment in the middle of the list may be stopping it, for whatever reason. Just some feedback.

__________________
Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code.
Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

deanosrs

What stops it is the need to look at the 2nd opcode in the list...

DaWei

What stops it are a number of things. For one thing, when you insert an item into the list, you set the next pointer equal to the current pointer. This means that when you are traversing the list via next pointers, you get to the end and go around in circles.

Your code is very clunky. By that, I mean that you have set out to solve all your problems by creating mechanisms of your own. This despite the fact that lebenty-zillion guys before you have had to deal with bad-nasties like unknown and undesired amounts of whitespace in the input. There are standard library functions that deal with the various kinds of most common input. I don't mean that to be a harsh critique. It's common among novices with experience in related areas. I also don't intend to address it at this time, although I might later. My immediate goal is to find out what the failure mechanisms are. A segmentation fault is one of them. Further, the initial loading up of isa is not working properly. News at 11:00.

__________________
Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code.
Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

grumpy

Phew. Go away for a few hours, and stuff happens.

InfoGeek. You're right; thanks for the correction.

deanosrs: I haven't downloaded your zip file this time, as Dawei seems to have looked at it, but my impression is you're tying yourself in knots quickly.

My suggestion: There comes a point with some bodies of code where the best thing to do is throw it away and start again. I think you've reached it. Put that code to one side, sit back and mull over the concepts, design it again, and then code a new one from scratch. It is not that big, and trying to find all the faults in it will probably take more time than coding a new one.

As to your posts....

The first line in your search function is this .....
which deliberately dereferences a pointer if it is NULL. The printf() call must go.

Two reasons. Firstly, NULL is a special value for initialising a pointer (on good quality compilers NULL will have a type that will generate an error if you try to use it to initialise something that is a non-pointer). The second and real reason is that struct types should be initialised with an initialiser list (i.e. instr listname = { <set of values> };) not with a single value.


Because the RHS on the third line is dereferencing a NULL pointer, and the result of that is undefined behaviour. One symptom of undefined behaviour (depending on your OS) is a segmentation fault.

deanosrs

Awesome I can't thank you guys enough for the help you're giving me.

Ok I'm going to take your advice Grumpy and redesign the insert into list/table functions. However before I do that I just need to make sure I fully understand so that I plan properly. Can you just tell me whether or not this stuff is correct:

assigns some memory somewhere for an "instr" and puts the address to this memory in list. so list is now just a pointer to that memory right?

creates an array of instr's somewhere in memory, all with no content.

this would then copy the string "add" into the opcode of list isa[1].

Ok so say I've created this table, how do I go about adding stuff to it? Surely I first find the right part of the array through generating a key first. Say the key is 1. So I then take the instr isa[1] list. So how do I check if there's anything in the list right now?

As for the whitespace stuff, could someone link me up to an online reference of built in C libraries? Thanks.

DaWei

Let me make one point here, before you get into the redesign. You define an instr, which is a struct holding certain needed entities. Very early on you actually set aside local memory for 23 of these. They are actually instantiated, but not initialized. This is done with the statement, "instr isa [23];". THIS MAKES 23 INSTRs. Not pointers to an inst -- an actual full blown inst. An array of instr pointers would be declared thusly: "instr *isa [23];". Then, when you decide you need an 'real' inst, you dynamically allocate it. You derive an index by hashing the opcode and try to store the POINTER to the dynamically allocated inst into the actual inst, itself. You force this to be accepted by the compiler by misrepresenting an argument type.

Either locally set aside memory for the instrs, or dynamically set aside memory for them and store the references to that dynamically allocated entity into a locally allocated array of POINTERS.

This code is just far too garfled up to patch the cracks with sealant. The support beams are hollow.

For your 'getNextLine', investigate fgets. To handle parsing the stuff in the line, investigate sscanf and strtok. To perform case conversions, investigate toupper and tolower. Also investigate such things as isspace, isdigit, isalpha, etc.

__________________
Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code.
Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

The Dark

Might I point you again to post #8? Change your array to
(The = {}; is to initialise it with NULLs}
Then change back all the code you had to add & and * to make it work with the instr isa[23]. (I have tried this, up to the point of reading the file and looked at th results in a debugger and it seemed to work fine).

What is happening is that when you are assigning into the array of objects, the C compiler is performing a copy of the whole object. This means that you cannot push the added element onto the start of the list by manipulating the next pointers as the start of the list is a fixed location.

grumpy

No, you're incorrect.

A declaration of
creates an unitialised pointer, and never creates something for that pointer to point at. The basic rule is that, if you want a pointer to actually point at something, you have to deliberately create (or find) an appropriate object and set the value of the pointer to be the address of that object. Dawei's pointer tutorial (one of the links in his signature) goes into that at some length.

The declaration
is a special case. It initialises list to be a NULL pointer. A NULL pointer is a specific value that identifies "this is not an object". No object is created for list to point at. And attempt to dereference a NULL pointer (eg access members of the instr type) yields undefined behaviour. The purpose of a NULL pointer is to allow things like this;
If list is NULL, then the access of list->opcode will yield undefined behaviour, and so will the printf() call. So we protect the call by making sure list is not NULL.

There are times when you can do without the (list != NULL) test in the above. In particular, if you have designed a function so that list is guaranteed to be a valid object, you don't need to do the test. That comes down to your program design.

It creates an array of instrs that exist somewhere in memory. They are unitialised, so your description of "no content" is incorrect. The do contain content, but you have no way of knowing what they contain. In practice, on a lot of compilers, the memory used for the isa array happens to be whatever was in that area of physical (or logical) memory before the compiler grabbed it and made it exclusively available to your program as the "isa" array.
Yes. As your opcode member is a string of 5 bytes, that's what it does.

If we do this;
The only thing you know about the isa array is that element 0 and elements 2..22 are uninitialised, and could contain anything. The first four characters of isa[1].opcode will be 'a', 'd', 'd', and '\0'. The fifth byte of isa[1].opcode (i.e. isa[1].opcode[4]) will be uninitialised, and therefore potentially contains anything. The members of isa[1], other than opcode, are uninitialised --- that does not mean they contain "nothing"; it means their content is unknown.
The short story is "however you define it".

If you have not initialised elements of your list, you can assume NOTHING about what they contain, so no test is possible. So you have to initialise your list before you can test anything (i.e. copy data to strings, assign integer members, etc etc). The reason for this is performance: the compiler doesn't want to waste resources initialising an object if your first step is going to be changing them to something else.

Once you've initialised the elements of your list, then you test based on the data you have shoved into the list elements. You need to define the convention. For example, you might choose to do;
if you want that to mean a particular element is empty. You would test this by something like
or
The first hit from a google search for "C standard library" yielded this