c struct help

[Robert Rota]

Need help figuring out why my program core dumps and if I have the
structures right. If you are interested in helping me I can send you a
copy of the code. The program is supposed to mimic a 3 level
pagetable.

Thank you.

struct _PageTable {
struct _PageTable **PageArray;
unsigned int *FrameArray;
};
typedef struct _PageTable PAGETABLE;


if (PageTable->PageArray[addresslvl0] = NULL) {
PAGETABLE *lvl1;
lvl1 = malloc(sizeof(PAGETABLE));
lvl1->PageArray = malloc(sizeof(PAGETABLE) * lvl1size);
int i;
for (i = 0; i < lvl1size; i++) {
lvl1->PageArray = NULL;
}
PageTable->PageArray[addresslvl0] = lvl1;
}

if (PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] = NULL)
{
PAGETABLE *lvl2;
lvl2 = malloc(sizeof(PAGETABLE));
lvl2->FrameArray = malloc(sizeof(unsigned int) * lvl2size);
int i;
for (i = 0; i < lvl2size; i++) {
lvl2->FrameArray = NULL;
}
PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] = lvl2;
}


PageTable->PageArray[addresslvl0]->PageArray[addresslvl1]->FrameArray[addresslvl2]
= Frame;

 

[Kevin Goodsell]

Need help figuring out why my program core dumps and if I have the
structures right. If you are interested in helping me I can send you a
copy of the code. The program is supposed to mimic a 3 level
pagetable.

Thank you.

struct _PageTable {

The identifier _PageTable is reserved for the implementation. You may
not use it. In general, never begin an identifier with an underscore.
Sometimes it's OK. Sometimes it's not. Identifiers beginning with and
underscore followed by another underscore or an uppercase letter are
never OK.

This is sufficient to cause a core dump (because it makes your program's
behavior undefined), but is probably not the actual cause.

struct _PageTable **PageArray;

Note the type of PageArray: pointer to pointer to _PageTable struct.

unsigned int *FrameArray;
};
typedef struct _PageTable PAGETABLE;


if (PageTable->PageArray[addresslvl0] = NULL) {

You cannot place executable statements at file scope. All executable
statements must be inside a function.

In the future, if you want help from this group, post a *complete*,
*minimal* program that demonstrates the problem. If we can't compile it,
our ability to help (not to mention our willingness to) is severely
diminished.

As for this line itself, there's a number of likely problems. I don't
know the type of PageTable, but I'm guessing it's PAGETABLE. I don't
know what you've done with it up to this point (since you didn't include
complete code), but I doubt you've allocated space for
PageTable->PageArray to point to, therefore you are dereferencing an
invalid pointer when you index it. Finally, you are *assigning* NOT
comparing. Even if the dereference is valid, this will modify something
you didn't mean to, and the result will always be false.

PAGETABLE *lvl1;
lvl1 = malloc(sizeof(PAGETABLE));

Consider using the c.l.c-approved idiom for malloc:

p = malloc(n * sizeof *p);

This is less error-prone.

That said, why would you use malloc for a single PAGETABLE? I can think
of some valid reasons, but I suspect you are just confused. Don't
dynamically allocate without reason.

PAGETABLE lvl1;

lvl1->PageArray = malloc(sizeof(PAGETABLE) * lvl1size);

Remember the type of PageArray that we noted earlier? Now you are
allocating space sufficient to store lvl1size PAGETABLEs. But PageArray
is NOT a pointer to PAGETABLE, so this is probably an error. Had you
used the c.l.c-approved malloc idiom, you would have avoided this problem:

lvl1.PageArray = malloc(lvl1size * sizeof *lvl1.PageArray);

(Note that I replaced -> with . in order to be consistent with the
change I made to the declaration of lvl1.)

int i;

You cannot mix declarations and executable code unless you are using
C99. Since very few C99 implementations exist (only 1 that I know of), I
suspect this is an error. If your compiler accepted this, then you
probably need to check its documentation to learn how to invoke it in
standard-conforming mode.

for (i = 0; i < lvl1size; i++) {
lvl1->PageArray = NULL;

With the incorrect malloc into lvl1.PageArray, this could very well
cause a core dump. You don't know whether PageArray points to enough
memory for lvl1size objects of type 'pointer to _PageTable'.

}
PageTable->PageArray[addresslvl0] = lvl1;
}

if (PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] = NULL)

Again, this is an assignment, not an equality test.

{
PAGETABLE *lvl2;
lvl2 = malloc(sizeof(PAGETABLE));

Don't malloc without a reason.

lvl2->FrameArray = malloc(sizeof(unsigned int) * lvl2size);

Use the c.l.c-approved idiom.

int i;

Can't mix declarations and code.

for (i = 0; i < lvl2size; i++) {
lvl2->FrameArray = NULL;

This is non-portable. NULL may be a (void) pointer type, and you are
assigning it to an unsigned int. This will cause a diagnostic on some
implementations.

}
PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] = lvl2;
}


PageTable->PageArray[addresslvl0]->PageArray[addresslvl1]->FrameArray[addresslvl2]
= Frame;

-Kevin

 

[Mark Gordon]

On 6 Dec 2003 09:07:26 -0800

Need help figuring out why my program core dumps and if I have the
structures right. If you are interested in helping me I can send you a
copy of the code. The program is supposed to mimic a 3 level
pagetable.

It would be far easier if you had provided a complete, compilable
example that demonstrated the problem. As it is, some of what I suggest
may be things that are not wrong with your actual code.

Firstly, you need

struct _PageTable {
struct _PageTable **PageArray;

In general you should avoid identifiers starting with underscores since
most of them are reserved for the implementation. It's easier to just
not use them at all that to bother learning when you can use which
identifiers starting with underscores.

unsigned int *FrameArray;
};
typedef struct _PageTable PAGETABLE;

Some people say that typdefing a struct is a bad things in general since
it hides the fact you are using a struct.

if (PageTable->PageArray[addresslvl0] = NULL) {

For all I know PageTable might not be initialised, of addresslvl0 might
point beyond the allocated memory.

PAGETABLE *lvl1;
lvl1 = malloc(sizeof(PAGETABLE));

lvl1 = malloc(sizeof *lvl1);

Would be better since it is less error prone. Although in this case you
have it right.

Also, you need to check if malloc succeeds, otherwise you will end up
dereferencing a NULL pointer if it fails..

lvl1->PageArray = malloc(sizeof(PAGETABLE) * lvl1size);

lvl1->PageArray = malloc(lvl1size * sizeof *lvl1->PageArray);

Note that in your struct definition there are two * before PageArray so
this is actually going to allocate a different amount of space.

int i;

Only valid if you are using a conforming C99 compiler or some strange
non-standard extension to an earlier version of C. We don't discuss
extensions to C, so declaring i at the start of the block would be
better.

for (i = 0; i < lvl1size; i++) {
lvl1->PageArray = NULL;
}
PageTable->PageArray[addresslvl0] = lvl1;

Again, we don't know what is going on here so we can't really comment.

}

if (PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] = NULL)

^^^
This is probably a problem, although not the problem.

{
PAGETABLE *lvl2;
lvl2 = malloc(sizeof(PAGETABLE));
lvl2->FrameArray = malloc(sizeof(unsigned int) * lvl2size);
int i;
for (i = 0; i < lvl2size; i++) {
lvl2->FrameArray = NULL;
}
PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] =
lvl2;
}


PageTable->PageArray[addresslvl0]->PageArray[addresslvl1]->FrameArray
[addresslvl2]= Frame;

There may be other problems, but I would normally use tools that would
tell me about problems as a first pass, and without complete code I
can't do that so I won't bother looking in more depth.

Please fix the identified problems and if it still doesn't work post a
small *complete* program, including the #includes at the start and a
definition of main, that exhibits the problem.

 

[Ben Pfaff]

struct _PageTable {

Most identifiers that begin with _ are reserved. *All*
identifiers that begin with _ followed by an uppercase letter are
reserved. Pick another identifier.

struct _PageTable **PageArray;
unsigned int *FrameArray;
};
typedef struct _PageTable PAGETABLE;


if (PageTable->PageArray[addresslvl0] = NULL) {

Should be ==.

PAGETABLE *lvl1;
lvl1 = malloc(sizeof(PAGETABLE));

When calling malloc(), I recommend using the sizeof operator on
the object you are allocating, not on the type. For instance,
*don't* write this:

int *x = malloc (128 * sizeof (int)); /* Don't do this! */

Instead, write it this way:

int *x = malloc (128 * sizeof *x);

There's a few reasons to do it this way:

* If you ever change the type that `x' points to, it's not
necessary to change the malloc() call as well.

This is more of a problem in a large program, but it's still
convenient in a small one.

* Taking the size of an object makes writing the statement
less error-prone. You can verify that the sizeof syntax is
correct without having to look at the declaration.

lvl1->PageArray = malloc(sizeof(PAGETABLE) * lvl1size);

See, here's an example of screwing up. You want
sizeof(PAGETABLE *), not sizeof(PAGETABLE). But if you wrote it
as sizeof *lvl1->PageArray, then it would be harder to screw up.

int i;

Mid-block declarations are only allowed in C99, but most
compilers don't support C99. Are you sure you know what language
standard you're writing to?

for (i = 0; i < lvl1size; i++) {

Also, if you're indeed using C99, then
for (int i = 0; i < lvl1size; i++)
is probably a better way to express it.

lvl1->PageArray = NULL;
}
PageTable->PageArray[addresslvl0] = lvl1;
}

if (PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] = NULL)

You mean ==, not =.

{
PAGETABLE *lvl2;
lvl2 = malloc(sizeof(PAGETABLE));
lvl2->FrameArray = malloc(sizeof(unsigned int) * lvl2size);
int i;
for (i = 0; i < lvl2size; i++) {
lvl2->FrameArray = NULL;
}
PageTable->PageArray[addresslvl0]->PageArray[addresslvl1] = lvl2;
}


PageTable->PageArray[addresslvl0]->PageArray[addresslvl1]->FrameArray[addresslvl2]
= Frame;

I think the rest of this is okay, but I haven't thought it
through carefully.