C passing an array of struct to delete one problems (from a newbie)

OK - please be patient for long winded explanation - I assume this is something just completely stupid on my part, because it's been (several) years since working in c. I have a couple of weird issues that I'm dealing with.

FIRST:

Have structure

Code:

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

struct ts{
  char *fname;
  char *lname;
  char *fingers;
  char *toes;
};

void delelement(char *, struct ts *);
int i;

int main(int argc, char **argv){
  struct ts *ex=(struct ts*)malloc(sizeof(struct ts));
  
  ex[0].fname="joe";
  ex[0].lname="bob";
  ex[0].fingers="11";
  ex[0].toes="9";

  ex[1].fname="billy";
  ex[1].lname="bronco";
  ex[1].fingers="10";
  ex[1].toes="10";

  ex[2].fname="martha";
  ex[2].lname="sue";
  ex[2].fingers="12";
  ex[2].toes="20";

  delelement("billy", ex);

  return 0;
}

now we get to the part I'm having problems with.
now for debugging I loop through and print out the values in the array of structs - this works (nevermind I'm not returning a value in this function - problem I'm running into is before we even get to that)

Code:

void delelement(char *delwhat, struct ts *passedex){

  //struct ts *tempex=(struct ts*)malloc(sizeof(struct ts));
  for(i=0; i<sizeof(passedex)-1; i++){
    printf("passedex[%d].fname is %s\n", i, passedex[i].fname);    
    printf("passedex[%d].lname is %s\n", i, passedex[i].lname);
    printf("passedex[%d].fingers is %s\n", i, passedex[i].fingers);
    printf("passedex[%d].toes is %s\n", i, passedex[i].toes);
  }
  return;
}

now THAT works fine - prints out information correctly.

now let's simply remove the comment and define the temporary array of structs

Code:

void delelement(char *delwhat, struct ts *passedex){

  struct ts *tempex=(struct ts*)malloc(sizeof(struct ts));
  for(i=0; i<sizeof(passedex)-1; i++){
    printf("passedex[%d].fname is %s\n", i, passedex[i].fname);    
    printf("passedex[%d].lname is %s\n", i, passedex[i].lname);
    printf("passedex[%d].fingers is %s\n", i, passedex[i].fingers);
    printf("passedex[%d].toes is %s\n", i, passedex[i].toes);

  }
  return;
}

BOOM - segfault
passedex[0].fname is joe
passedex[0].lname is bob
passedex[0].fingers is 11
passedex[0].toes is 9
passedex[1].fname is billy
Segmentation fault

OK so I tried a different approach - which kinda works

Code:

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

struct ts{
  char *fname;
  char *lname;
  char *fingers;
  char *toes;
};

void delelement(char *, struct ts *, struct ts *);
int i;

int main(int argc, char **argv){
  struct ts *ex=(struct ts*)malloc(sizeof(struct ts));
  struct ts *tempex=(struct ts*)malloc(sizeof(struct ts));
  
  ex[0].fname="joe";
  ex[0].lname="bob";
  ex[0].fingers="11";
  ex[0].toes="9";

  ex[1].fname="billy";
  ex[1].lname="bronco";
  ex[1].fingers="10";
  ex[1].toes="10";

  ex[2].fname="martha";
  ex[2].lname="sue";
  ex[2].fingers="12";
  ex[2].toes="20";

  delelement("billy", ex, tempex);

  return 0;
}

void delelement(char *delwhat, struct ts *passedex, struct ts *tempex){

  //struct ts *tempex=(struct ts*)malloc(sizeof(struct ts));

  for(i=0; i<sizeof(passedex)-1; i++){
    printf("passedex[%d].fname is %s\n", i, passedex[i].fname);    
    printf("passedex[%d].lname is %s\n", i, passedex[i].lname);
    printf("passedex[%d].fingers is %s\n", i, passedex[i].fingers);
    printf("passedex[%d].toes is %s\n", i, passedex[i].toes);
  }
  return;
}

WORKS fine... (tempex now defined in main)
passedex[0].fname is joe
passedex[0].lname is bob
passedex[0].fingers is 11
passedex[0].toes is 9
passedex[1].fname is billy
passedex[1].lname is bronco
passedex[1].fingers is 10
passedex[1].toes is 10
passedex[2].fname is martha
passedex[2].lname is sue
passedex[2].fingers is 12
passedex[2].toes is 20


now lets start assigning values to *tempex - no segfault with tempex defined in main

Code:

void delelement(char *delwhat, struct ts *passedex, struct ts *tempex){

  //struct ts *tempex=(struct ts*)malloc(sizeof(struct ts));

  for(i=0; i<sizeof(passedex)-1; i++){
    printf("passedex[%d].fname is %s\n", i, passedex[i].fname);    
    printf("passedex[%d].lname is %s\n", i, passedex[i].lname);
    printf("passedex[%d].fingers is %s\n", i, passedex[i].fingers);
    printf("passedex[%d].toes is %s\n", i, passedex[i].toes);
    tempex[i].fname=passedex[i].fname;
    tempex[i].lname=passedex[i].lname;
    tempex[i].fingers=passedex[i].fingers;
    tempex[i].toes=passedex[i].toes;
  }
  return;
}

but NOW - weirdness

passedex[0].fname is joe
passedex[0].lname is bob
passedex[0].fingers is 11
passedex[0].toes is 9
passedex[1].fname is billy
passedex[1].lname is bronco
passedex[1].fingers is joe
passedex[1].toes is bob
passedex[2].fname is 11
passedex[2].lname is 9
passedex[2].fingers is billy
passedex[2].toes is bronco


obviously I'm just missing something stupid, or understanding this wrong, but have now dug a rut that I can't get out of. Any help would be appreciated.

The goal is to have a dynamic array of structures containing char *'s. Once past this issue, there will be an instance in main (or wherever) that I wish to delete one of those structures.

What I was going for was something like...

Code:

struct ts* delelement(char *delwhat, struct ts *passedex, struct ts *tempex){

  //struct ts *tempex=(struct ts*)malloc(sizeof(struct ts));

  for(i=0; i<sizeof(passedex)-1; i++){
    printf("passedex[%d].fname is %s\n", i, passedex[i].fname);    
    printf("passedex[%d].lname is %s\n", i, passedex[i].lname);
    printf("passedex[%d].fingers is %s\n", i, passedex[i].fingers);
    printf("passedex[%d].toes is %s\n", i, passedex[i].toes);
    //load tempex with everything except the one I want to delete
    if(!(passedex[i].fname==delwhat)){
      tempex[i].fname=passedex[i].fname;
      tempex[i].lname=passedex[i].lname;
      tempex[i].fingers=passedex[i].fingers;
      tempex[i].toes=passedex[i].toes;
    }
  }
  free(passedex); //haven't got here yet - dunno if needed

  for(i=0; i<sizeof(passedex)-1; i++){
    passedex[i].fname=tempex[i].fname;
    passedex[i].lname=tempex[i].lname;
    passedex[i].fingers=tempex[i].fingers;
    passedex[i].toes=tempex[i].toes;
  }
  
  return passedex;
}

so it would create (or have) a temporary array of structs to work with... load that array minus the one to be deleted... reload the passed array of structs and pass it back.

Code:

  struct ts *ex=(struct ts*)malloc(sizeof(struct ts));
  
  ex[0].fname="joe";
  ex[0].lname="bob";
  ex[0].fingers="11";
  ex[0].toes="9";

  ex[1].fname="billy";
  ex[1].lname="bronco";
  ex[1].fingers="10";
  ex[1].toes="10";

  ex[2].fname="martha";
  ex[2].lname="sue";
  ex[2].fingers="12";
  ex[2].toes="20";

How many struct you allocated.

Yep... you're missing 2 things...

1) sizeof(passedex) does not return the size of the array, it returns the size of the *pointer* you passed into the function. You just got lucky because the array is only 3 elements.

2) This is C... you cannot assign strings with the = sign, what you actually end up doing is assigning the pointer to the string. Use strcpy() and the other functions in the string library to manipulate string contents.

It would be a lot easier for you to redefine your struct like this...

Code:

struct ts{
  char fname[20];
  char lname[20]
  int fingers;
  int toes;
};

and work on it with the functions from string.h.

O_o

There is so much wrong with that code I don't know where to begin.

You need to just drop this for now, pull back, and start with learning how memory allocation works following up with how arrays decay into pointers even at the best of times.

You are playing with memory you don't own, and `sizeof(passedex)' is a constant probably equal to four or eight.

Soma

Originally Posted by phantomotap

O_o

There is so much wrong with that code I don't know where to begin.

And this is helpful, exactly how?

was going for dynamic allotment

at first I was approaching this whole thing with a multidimensional char *

such as char *people[][3];

This is where I got into a rut and turned to the struct. I forget now what the exact issue was with the multidimensional array, but there was some issue where I instead turned to a struct of char *'s.

What I am wanting to achieve is to get some sort of global "tracking" multidimentional array, or array of structs that will hold 4 bits of information. The goal is to be able to access this from anywhere across the board, and to be able to delete one and "squeeze" the others into that space... so if I have

joe, bob, 11, 9
billy, bob, 10 ,10
mary, sue, 12, 14

and then delete billy bob... i do not have

joe, bob, 11, 9
null, null, null, null
mary, sue, 12, 14

but have

joe, bob, 11, 9
mary, sue, 12, 14


Going for dynamic allocation because at any time there can be any number of "people"

Originally Posted by snowolfe

was going for dynamic allotment

at first I was approaching this whole thing with a multidimensional char *

such as char *people[][3];

This is where I got into a rut and turned to the struct. I forget now what the exact issue was with the multidimensional array, but there was some issue where I instead turned to a struct of char *'s.

What I am wanting to achieve is to get some sort of global "tracking" multidimentional array, or array of structs that will hold 4 bits of information. The goal is to be able to access this from anywhere across the board, and to be able to delete one and "squeeze" the others into that space... so if I have

joe, bob, 11, 9
billy, bob, 10 ,10
mary, sue, 12, 14

and then delete billy bob... i do not have

joe, bob, 11, 9
null, null, null, null
mary, sue, 12, 14

but have

joe, bob, 11, 9
mary, sue, 12, 14


Going for dynamic allocation because at any time there can be any number of "people"

Dynamic allocation of the structs --probably in a linked list-- is a fairly good way to handle this.

However; dynamic allocation inside the structs is going to be frought with problems and unless you are extremely careful about free() -ing things it's almost a certainty you will run into memory management problems -- so called "memory leaks".

In a linked list it's fairly easy to delete one element... just connect the one previous to the next one and free() the current struct. Adding can be done at beginning or end with approximately equal ease.

But always keep that struct as an intact entity... dynamic strings inside structs do work but they complicate the bejeebers out of everything.

Originally Posted by CommonTater

2) This is C... you cannot assign strings with the = sign, what you actually end up doing is assigning the pointer to the string.

That's called a string literal and it's okay:

Code:

struct ts {
	char *fname;       // better: const char*
	char *lname;
	int fingers;
	int toes;
};

struct ts eg[3] = { 
	{ .fname = "billy", .lname = "bob", .fingers = 3, .toes = 4 }, 
	{ .fname = "billy", .lname = "joe", .fingers = 4, .toes = 3 }, 
	{ .fname = "mary", .lname = "brown", .fingers = 11, .toes = 256 }
};

Just do not try and change the value of fname or lname.

Originally Posted by MK27

That's called a string literal and it's okay:
Just do not try and change the value of fname or lname.

... or assign strings from user input or write the struct to disk.

Seriously... you don't think he's going to stay with his little test program do you?
At some point he's going to want to manipulate those strings, at which point the whole thing will come crashing down....

O_o

Writing the data to a disk wouldn't be a problem.

[Edit]
Oh, you edited your post.

Well, if we are going to start talking about serializing data and real programs, then he is going to need to use allocation for those data members as well which you took care to warn him from.
[/Edit]

Soma

Originally Posted by phantomotap

O_o

Writing the data to a disk wouldn't be a problem.

Soma

I said "the struct"... not the data.


Writer program....

Code:

#include <stdio.h>

struct t_Test
  { char * name;
     int x; }
  Test;

int main (void)
  {  File* file;

      Test.name = "Fred Smith";
      Test.x = 20;

      file  = fopen("DiskTest.dat","wb");
      fwrite(&Test, sizeof(Test), 1, file);
      fclose(file);
      return 0; }

Reader program...

Code:

// writer program
#include <stdio.h>

struct t_Test
  { char * name;
     int x; }
  Test;

int main (void)
  {  File* file;

      file  = fopen("DiskTest.dat","rb");
      fread(&Test, sizeof(Test), 1, file);
      fclose(file);

      printf("Name : %s\nX : %d\n,Test.name, Test.x);  
      return 0; }

OOPS! What happened to the name?

Take a look at the disk file in a hex editor... do you see the name in there anyplace?

Now revise the code so that you have char name[20]; in both programs and use strcpy() to assign "Fred Smith" in the writer program. What happens then?

Currently looking into linked list.

Let me see if I can explain better what's happening, and what exactly I'm trying to achieve.

I have a loop that is hit on each packet coming in over ethernet. (Using libpcap to capture packets - which have all this already going).

Now there is a processing function that gets hit in the case of it being a tcp packet.

So we have ...

Code:

packet_data_loop(...){
...
//found tcp
  packet_process_function(...);
}

packet_process_function(...){
//process stuff - checking a certain byte/combination of bytes to determine specifics on what this particular packet is doing
}

Now - in the packet_process_function, if I find a certain "service", I want to add some info such as size, destination port, prog name, file name to be "tracked" AND be accessed by the packet_data_loop (or any other functions for that matter)

Now back in the packet_data_loop - Everytime it gets a packet, I want to check this "global" array of data... and IF i have anything, match it to the destination port. IF I have a match then I know to call a different packet_process_function because I want to completely handle this particular bit of data coming in differently.

Now that second packet_process_function(2) will already know what the size is, so each time it hits, it writes to disk. Once it comes to a point that the filesize is >= the size stored in the global tracking, it can move the file, name it accordingly, and then remove that entry from tracking.

I hope that's not too much gibberish to follow :P

And thanks for the help so far. Like I said - it's been yeeeeaaaaarrrrrsss for me.

Code:

#include <stdio.h>

struct t_Test
  { char * name;
     int x; }
  Test;

int main (void)
  {  File* file;

      Test.name = "Fred Smith";
      Test.x = 20;

      file  = fopen("DiskTest.dat","wb");
      fwrite(&Test, sizeof(Test), 1, file);
      fclose(file);
      return 0; }

Code:

// writer program
#include <stdio.h>

struct t_Test
  { char * name;
     int x; }
  Test;

int main (void)
  {  File* file;

      file  = fopen("DiskTest.dat","rb");
      fread(&Test, sizeof(Test), 1, file);
      fclose(file);

      printf("Name : %s\nX : %d\n,Test.name, Test.x);  
      return 0; }

O_o

You know what, I should be ashamed. I knew that you weren't as experienced as you claim. I figured you for another arrogant "intermediate". The types who "Only ever needs help that one time!". The fun of tagging is in prodding those types. I didn't know that you were that much of a newbie. I didn't realize that you truly didn't get the joke. I apologize.

I will still correct you when you are wrong, as I would anyone else, but I will not tag you anymore. I can't promise I will not call you "Dude" because I play online games with a lot of teenagers.




Now then, your examples are flawed because you have only applied a simple binary dump. Writing string literals to a file isn't a problem. It doesn't work because you haven't done the programming necessary to make it work.

Reading strings is also not a problem, but you'll have to do the memory management and clone string literals on copy and assignment operations just as would if you were going to make a mutable copy.

The C file "IO" routines don't know about custom data layouts. You have to program serialization manually. This is true for dumb structures with pointers to character arrays as it would be for a linked list.

Soma

I won't pretend to completely understand at this point but I think I get the gist of it...

You may also want to look into a random access file for this as well... They're easy enough to write; based on reading/writing structs in specific file locations. You could then use an in memory index to track which record is in which slot in the file... really fast if done correctly and only requires 1 struct in memory at a time.

I was mainly concerned that you didn't end up writing pointers to strings to your disk. The bugaboo is, of course, they probably won't still exist when you reload the record... so any text in your structs winds up being garbaged.

Now - in the packet_process_function, if I find a certain "service", I want to add some info such as size, destination port, prog name, file name to be "tracked" AND be accessed by the packet_data_loop (or any other functions for that matter)

Now back in the packet_data_loop - Everytime it gets a packet, I want to check this "global" array of data... and IF i have anything, match it to the destination port. IF I have a match then I know to call a different packet_process_function because I want to completely handle this particular bit of data coming in differently.

Now that second packet_process_function(2) will already know what the size is, so each time it hits, it writes to disk. Once it comes to a point that the filesize is >= the size stored in the global tracking, it can move the file, name it accordingly, and then remove that entry from tracking.

^_^

Are you writing a download manager?

Anyway, considering the state of your program, I'd say you are a long way from managing whatever it is you are doing. Why not grab a C implementation of a linked list and see if you can follow it? Better yet, why not write one yourself first because that will surely help with your memory management problems.

In any event, a tree indexed with the most relevant data is probably your best bet.

Soma