Help with accessing C type data structures with nasm

[jaswax]

It has been a while since I have worked on my OS. I recently got back to it and have made some progress but I seem to have hit a wall.

Info:
OS Developed on Ubuntu Linux
using gcc, ld and nasm

I have the following type of structure defined:

Code: Select all

typedef struct userinfo
{
  char *username;
  unsigned int rights;     
  unsigned int lastloggedin;
  unsigned int locked; 
} USERINFO;

I have the following c code:

Code: Select all

int main (void)
{
  USERINFO *userlogin;

  userlogin = openfile("user.nfo");

  printf (userlogin->username);
  printf ("User rights: %d",userlogin->rights);
  printf ("User last logged in: %d",userlogin->lastloggedin);
  printf ("User rights: %d",userlogin->rights);
  printf ("Is the user locked out?: %d",userlogin->locked);
}

openfile use a routine written in nasm that pulls the data from user.nfo into a buffer. A pointer to the buffer is then returned by openfile and stored in userlogin.

The issue that I am having is with displaying the username. When I printf the username I either get nothing or I get garbage. I have no problems with the data for the other members of the struct.

I have written an elf executable written in c that I used to create the file user.nfo. I have also made a test program written in c that I used to see if I can read back the same data that I wrote into my user.nfo file These actions where done under linux. The tests show that under linux I am able to get back the data that I wrote to the user.nfo file and can access all data elements of the struct USERINFO but this is not true with all elements under my OS as described above.

Any ideas with this or help is much appreciated.

[hrniels]

Could you please post the implementation of openfile and perhaps the assembler-routine, too?

Another possibility is that printf doesn't work correctly. Have you checked the values you get with a different approach? E.g. with a debugger.

[qw]

My guess is that the functions reads the user name (a string) into userlogin->username (a pointer), clobbering the entire structure. To be sure, we need to see the code for openfile.

[Troy Martin]

My guess is that the functions reads the user name (a string) into userlogin->username (a pointer), clobbering the entire structure. To be sure, we need to see the code for openfile.

You're right. A char* is generally only 4 (or bytes long, being a pointer, so you'd want to point the pointer to the filename section of the buffer.

[jaswax]

So as to not leave you hanging:

I have to go to work now. I will post the requested code when I get back.

Thank you for the replies so far.

[jaswax]

Ok, I am back. Long work day - 12 hours!

I went back and looked at my code that writes the data to user.nfo. I think what actually happened is that the pointer and data for my structure was still in memory even thou it was also on disk so that when I tested it it only looked liked it was working. My code to read and write was in the same executable. So when I went to read the data the I wrote from the file only the pointer to username was there and not the actual name. I wrote a separate test program under linux to only read the data from the file and I get the same results as in my os.

Here is the code:

Code: Select all

#include <stdio.h>
#include <stdlib.h>

#include "userlib.h"

int main(void)
{
  USERINFO      *userlogin;
  FILE                  *fp;

  uselogin = malloc(sizeof(USERINFO));

  fp = fopen ("user.nfo", "w");
  if (fp == NULL)
  {
     printf ("Can't open drive for output.\n");
     exit (2);
  }

  userlogin->username = "Joe";
  userlogin->rights = 4;
  userlogin->lastloggedin = 1527;
  userlogin->locked = 0;

  fwrite (userlogin, sizeof(USERINFO), 5, fp);

  fclose (fp);

  fp = fopen ("user.nfo", "r");
  if (fp == NULL)
  {
     printf ("Can't open drive for output.\n");
     exit (2);
  }

  fread (userlogin, sizeof(USERINFO), 5, fp);

  printf ("Current User: %s\n",userlogin->username);

  printf ("The time last logged in was: %d\n",userlogin->lastloggedin);

  fclose (fp);

  return 0;
}

I think that what I don't understand is how to save the actual data pointed to by username to the file user.nfo. I know that only the pointer to the data in memory is being saved in the file.

All of the above routines are from stanard libc and not libs that I have rewritten for my os.

So now my questions is how do I save the data to the file and not the pointer to the data?

[qw]

The easiest way is to make username an array of fixed size, though this limits the length of the user name:

Code: Select all

#define MAX_USERNAME_LENGTH 12 /* Or any other reasonable value */

typedef struct userinfo
{
  char username[MAX_USERNAME_LENGTH + 1]; /* Plus terminating null character */
  unsigned int rights;     
  unsigned int lastloggedin;
  unsigned int locked; 
} USERINFO;

Also change this:

Code: Select all

userlogin->username = "Joe";

to:

Code: Select all

strcpy(userlogin->username, "Joe");

Just make sure that "Joe" does not exceed MAX_USERNAME_LENGTH characters.

[hrniels]

I see two ways:

1. You store the string at the end of struct with a variable length (which you save, too). For example:

   Code: Select all

   typedef struct userinfo
   {
     unsigned int rights;
     unsigned int lastloggedin;
     unsigned int locked;
     unsigned int userLen;
     char username[];
   } USERINFO;

   (Alternatively "username[1]", depending on wether you want to use the gcc-extension or not)
2. You use a max-length. E.g.:

   Code: Select all

   typedef struct userinfo
   {
     char username[MAX_USER_LEN];
     ...

[qw]

Hrniels,
Option 1 would break sizeof (USERINFO).

Jaswax,
Another option is to use fprintf() and fscanf() instead of fwrite() and fread(). This will save the data in readable format. (EDIT: readable by humans.)

BTW the third argument to fread() and fwrite() is NOT the number of items in your structure! It now tells to read and write five structures instead of one. This way, you may destroy your heap. You should change 5 into 1.

Roel

[hrniels]

Hrniels,
Option 1 would break sizeof (USERINFO).

Sure. But this way you have a variable length for the name. Depends on your needs whats the better option...

[Owen]

type something[] at the end of a structure isn't a GCC extension; it's C99. GCC has an (obsolete) extension which does the same thing whereby you specify [0] instead, however, that's just preserved for backwards compatibility.

[hrniels]

type something[] at the end of a structure isn't a GCC extension; it's C99. GCC has an (obsolete) extension which does the same thing whereby you specify [0] instead, however, that's just preserved for backwards compatibility.

Ah, right. I mixed that up

[Solar]

Ad-hoc. I didn't test this.

Code: Select all

#include <stdio.h>
#include <stdlib.h>

#include "userlib.h"

int main(void)
{
  USERINFO      *userlogin;
  FILE                  *fp;
  const char * username = "Joe";

  userlogin = malloc( sizeof( USERINFO ) );  // Typo in your original code

  fp = fopen ("user.nfo", "w+");
  if (fp == NULL)
  {
     printf ("Can't open drive for output.\n");
     exit (2);
  }

  userlogin->username = malloc( strlen( username ) + 1 ); // +1 for terminating \0
  strcpy( userlogin->username, username );
  userlogin->rights = 4;
  userlogin->lastloggedin = 1527;
  userlogin->locked = 0;

  fprintf( fp, "%d|%s|%d|%d|%d\n", strlen( username ), userlogin->username, userlogin->rights, userlogin->lastloggedin, userlogin->locked );

  rewind(fp);
  free( userlogin->username );
  userlogin->rights = 0;
  userlogin->lastloggedin = 0;
  userlogin->locked = 0;

  int i;
  if ( fscanf( fp, "%d|", &i ) == 0 )
  {
     puts( "Data corruption!" );
     exit( 2 );
  }

  userlogin->username = malloc( i + 1 );
  if ( userlogin->username == NULL )
  {
     puts( "Out of memory!" );
     exit( 2 );
  }
  fgets( userlogin->username, i, fp );
  if ( fscanf( fp, "|%d|%d|%d\n", &userlogin->rights, &userlogin->lastloggedin, &userlogin->logged ) != 3 )
  {
    puts( "Data corruption!" );
    exit( 2 );
  }

  printf ("Current User: %s\n",userlogin->username);
  printf ("The time last logged in was: %d\n",userlogin->lastloggedin);

  fclose (fp);
  return 0;
}

[jaswax]

Thanks for all of the replies. I experimented with everyone's suggestions. I largely used Solar's idea.

The only other thing I would like to know is if there is a way to get the information to align on word boundaries? Not critical but I would like to know.