Linker for assembly object files

[Dewey1040]

I've attempted to create a linker for assembly files, what the program does is not the issue here though, I havent used pointer arithmetic in a few years and am definitely rusty in doing so. I'm getting a segmentation fault when I attempt to run the file and would like to at least make a few strides in figuring out why. My code is currently as follows, any input on the pointer arithmetic errors would be helpful, and if you need to know more on what the input files need to look like or what the output needs to look like I can give that information.

The struct might also be a little off... so insight on how to clean things up a bit would be helpful just to make things look prettier.

The program compiles fine with no warnings or errors.

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

typedef struct nament {
        char name[26];
        int addr;
        struct nament *next;
        } SYMTABENTRY;

void add_symbol( char *symbol, int linenum );
char* generate_code( int );
void append_table( void );
int search_table( char *symbol );

SYMTABENTRY *symtab = NULL;
SYMTABENTRY *finder = NULL;
FILE *p1, *p2, *p3;
char cstr_21[18];
int main( int argc, char *argv[] ) {
  int start = 1, end, pc_offset = 0, pc = 0;
    int linenum, dump_tab = 1;
    int new_pc, exit = 0;
    char instruction[18];
    char symbol[26];
    char executable[18];

    symtab = (SYMTABENTRY *)malloc(sizeof(SYMTABENTRY));

    if( strcmp( argv[argc - 2], ">" ) == 0){
      end = argc - 3;
      dump_tab = 0;
    }
    p1 = fopen( "/tmp/passone", "w+" );
    unlink( "/tmp/passone" );

    p3 = fopen( argv[argc-1], "w" );

    for( start = 0; start < end; start++ ){
         p2 = fopen("argv[start]","r");
         if( argv == NULL ) break;

     while( fscanf( p2, "%d %s", &pc, &instruction[0] ) != EOF) {

       if( instruction[0] == 'x' ){
         while( fscanf( p2, "%s %d", &symbol[0], &linenum) != EOF){
           add_symbol( symbol, linenum + pc_offset );
         }
         exit = 1;
       }
       if(exit == 1)
         break;

       new_pc = pc + pc_offset;
       symbol[0] = '\0';

       if( instruction[0] == 'U' ){
         fscanf( p2, "%s", &symbol[0]);
         fprintf( p1, "%d %s %s", new_pc, instruction, symbol);
       }
       else
         fprintf( p1, "%d %s", new_pc, instruction );
     }

     pc_offset = new_pc;

         fclose( p2 );
    }

    fseek( p1, 0, SEEK_SET );

    while( fscanf( p1, "%d %s", &linenum, &instruction[0] ) != EOF ){

      symbol[0] = '\0';

      if( instruction[0] == 'U' ){
    fscanf( p1, "%s", &symbol[0] );
    executable[0] = atoi(generate_code( search_table( symbol ) ));
    if(dump_tab){
      printf( "%s\n", executable );
    }
    else{
      fprintf( p3, "%s\n", executable );
    }
      }
      else{
    if(dump_tab){
      printf( "%s\n", instruction );
    }
    else{
      fprintf( p3, "%s\n", instruction );
    }
      }
    }

    return 0;
}

void add_symbol( char *symbol, int linenum ){

  append_table();

  finder->name[0] = atoi(symbol);
  finder->addr = linenum;
  finder->next = NULL;

}

void append_table(){

  finder = symtab;

  if( finder != NULL )
    while( finder->next != NULL )
      finder = finder->next;

  finder->next = (SYMTABENTRY *)malloc( sizeof( SYMTABENTRY ));
}

int search_table( char* symbol ){

  finder = symtab;

  if( finder != NULL ){
    while( finder->next != NULL ){
      if( strcmp( symbol, finder->name ) == 0 )
    return( finder->addr );
    }
  }
  printf( "Unrecognized token %s", symbol );

  exit(1);

}

char* generate_code( int linenum ){

  int i = 17, tmp;

  char binnum[18];

  binnum[17] = '\0';

  for( i = 17; i >= 0; i--){
    tmp = linenum % 2;
    binnum[i] = tmp;
    linenum /= 2;
  }

  cstr_21[0] = atoi(binnum);
  return cstr_21;
}

[Ancient Dragon]

line 8: I'd make that an unsigned int instead of int because addresses should never be negative number.

line 41 p2 = fopen("argv[start]","r");

remove the quotes from around argv[start] -- the compiler thinks that is the actual filename, not a variable that contains the file name.

lines 35 and 36: Why are you opening a file then attempting to immediately delete it? unlink() will fail if the file is open.

line 44: Remove the spaces in the format specifier, e.g. it should be "%d%s". There should not be spaces between % and d or s.

line 44: Yes, the same line as above. The = EOF at the end should be == EOF because you want the boolean operator not the assignment operator.

line 104: That line is confusing. Why are you tring to store an integer in the first byte of a character array? That may work if the value of the integer is less than 126, which is the largest positive value that can be stored in a char data type. And is the first few bytes of variable symbol all numeric digits?

There are more than likely lots of other problems with that program I have not found.

[Dewey1040]

i fixed line 8 and 41, I use unlink so no other processes can use that temporary file... just something my professor said to use... Im not sure why. "The unlink function deletes the file name filename. If this is a file's sole name, the file itself is also deleted. (Actually, if any process has the file open when this happens, deletion is postponed until all processes have closed the file.)" The part in parenthesis is what I take advatage of for this.

The .o files I'm reading in look something along the lines of this:

   0  U0000000000000000    arg1:
   1  1111010000000000
   2  U0000000000000000    arg2:
   3  1111010000000000
   4  U1110000000000000    myadd:
   5  U0001000000000000    rslt:
   6  1111111111000000
   10  0000000000011001
   11  0000000001001011
   12  0000000000000000
  4096 x
    rslt:                       12
    arg2:                       11
    arg1:                       10
    main:                        0

Should I still remove the spaces for %d and %s? and I'm using != EOF... not just =, because Im scanning until I reach EOF.

In regards to 104 thats where I was forgetting correct pointer syntax. I'm trying to just make the name of the current node equal to the string passed as symbol, this is the line of code I came up with to satisfy gcc to not give me an error or warning. Theres a few other lines of code throughout the program which attempt to do similar things and all have the same syntax.

[Ancient Dragon]

if any process has the file open when this happens, deletion is postponed until all processes have closed the file.)" The part in parenthesis is what I take advatage of for this.

That is a BAD habbit to get into. That behavior doesn't work with all operating systems. For example remove() doesn't work like that on MS-Windows. Don't attempt to delete the file until AFTER you are finished with it and the file has been closed.

Should I still remove the spaces for %d and %s?

Yes. There should be no spaces between the % symbol and 'd' or 's'.

in regards to 104 thats where I was forgetting correct pointer syntax.

you want to call strcpy() to copy the string into name: strcpy(finder->name, symbol);

[Dewey1040]

okay so I cleaned the program up a little bit, but line 104 is definitely where I'm getting the segmentation fault currently. My current line reads :

while( fscanf( p2, "%d%s", &pc, &instruction[0] ) != EOF) {

I need to read in an integer into pc... I'm sure thats not the issue, then I need to read in the following 16 or 17 char string into instruction and I'm fairly certain thats where the seg fault is coming into play

And in regards to using unlink, its specifically for a program in unix, but I'll make a mental note of it being a bad habit.

[Ancient Dragon]

Are you certain p2 is an open FILE pointer?

[Dewey1040]

Yeah I was being dumb and was including main.o instead of main1.o... thanks for the help Ancient Dragon!