c struct help

Discussion in 'C Programming' started by Robert Rota, Dec 6, 2003.

  1. Robert Rota

    Robert Rota Guest

    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;
     
    Robert Rota, Dec 6, 2003
    #1
    1. Advertising

  2. Robert Rota wrote:

    > 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
    --
    My email address is valid, but changes periodically.
    To contact me please use the address from a recent posting.
     
    Kevin Goodsell, Dec 6, 2003
    #2
    1. Advertising

  3. Robert Rota

    Mark Gordon Guest

    On 6 Dec 2003 09:07:26 -0800
    (Robert Rota) wrote:

    > 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

    #include <stdlib.h>

    > 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.
    --
    Mark Gordon
    Paid to be a Geek & a Senior Software Developer
    Although my email address says spamtrap, it is real and I read it.
     
    Mark Gordon, Dec 6, 2003
    #3
  4. Robert Rota

    Ben Pfaff Guest

    (Robert Rota) writes:

    > 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.
    --
    int main(void){char p[]="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz.\
    \n",*q="kl BIcNBFr.NKEzjwCIxNJC";int i=sizeof p/2;char *strchr();int putchar(\
    );while(*q){i+=strchr(p,*q++)-p;if(i>=(int)sizeof p)i-=sizeof p-1;putchar(p\
    );}return 0;}
     
    Ben Pfaff, Dec 6, 2003
    #4
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. RA Scheltema
    Replies:
    3
    Views:
    404
    RA Scheltema
    Jan 6, 2004
  2. Gunnar G

    struct in struct

    Gunnar G, May 31, 2004, in forum: C++
    Replies:
    14
    Views:
    806
  3. DanielEKFA
    Replies:
    8
    Views:
    616
    DanielEKFA
    May 16, 2005
  4. James Harris
    Replies:
    4
    Views:
    1,396
    James Harris
    Oct 9, 2003
  5. Chris Fogelklou
    Replies:
    36
    Views:
    1,391
    Chris Fogelklou
    Apr 20, 2004
Loading...

Share This Page