C - Char-peker endrer seg?

[Sleggefett]

Driver med noen skoleoppgaver, og føler jeg er rimelig godt på vei men har nå truffet et kinky problem, at en char-peker ikke blir satt korrekt(?). Kompilering med -Wall og -Wextra går helt fint, itillegg stemmer det jeg har skrevet ganske greit overens med fasit(som jeg så klart ikke sjekka før jeg gikk videre til neste oppgave). Her er de tre mest relevante delene av koden, slik jeg ser det;

 

 

void pers_setName(pers *p, char *n)
{
  if(strlen(n) > MAX_LENGTH_NAME) {
    printf("Length is over 30 characters for name:\n%s\n", n);
    printf("Name is not set.\n");
  }
  p->name = n;
}

char* pers_getInfo(pers *p)
{
  char *returVal = malloc(sizeof(p->age) + sizeof(p->name));

  if((pers_checkSetName(p)) && (pers_checkSetAge(p))) {
    sprintf(returVal, "%s:%d", pers_getName(p), pers_getAge(p));
  } else {
    sprintf(returVal, "Name and/or age not set");
  }

  return returVal;
}

pers* pers_setInfo(char *setInfo)
{
  pers *a = pers_construct(); //Creates pers-object

  char line[1 + strlen(setInfo)]; //Creates char-array
  strcpy(line, setInfo); //Copies pers-info into array

  char *name = strtok(line, ":"); //Gets persname from passed info
  char *age = strtok(NULL, ":"); //Gets persage from passed info

  //Can't set variables if they're null
  if(name == NULL || name == NULL) {
    return NULL;
  }

  //Manual atoi, just for fun
  unsigned int i;
  int ageI = 0;
  int mod = 1;

  for(i = 0; i < strlen(age); i++) {
    if(age[i] >= '0' && age[i] <= '9') {
      ageI = ageI * 10 + age[i] - '0';
    } else if(age[i] == '-' && i == 0) {
      mod = -1;
    }
  }
  ageI *= mod; //Modifier if age <0
  //End manual atoi

  pers_setName(a, name);
  pers_setAge(a, ageI);
  //printf("%s\n", a->name);
  return a;
}

int main()
{
  //Creates pers-objects as well as initialize c with customvalues
  pers *a = pers_construct();
  pers *b = pers_construct();
  char *arba = "Tango:20";
  pers *c = pers_setInfo(arba);

  //printf("%s\n", c->name);
  //Sets values for a and b
  pers_setName(a, "Kalle");
  pers_setName(b, "Klovn");
  pers_setAge(a, 20);
  pers_setAge(b, 21);

  //Gets the personinfo 
  char *aInfo = pers_getInfo(a);
  char *bInfo = pers_getInfo(b);
  char *cInfo = pers_getInfo(c);
  printf("%s\n", aInfo);
  printf("%s\n", bInfo);
  printf("%s\n", cInfo);

  free(a);
  free(b);
  free(c);
  free(aInfo);
  free(bInfo);
  free(cInfo);
  return 0;
}

 

 

Så vidt jeg ser er det char-pekeren i pers c som driver å tuller. cInfo gir div. merkelige tegn etterfulgt av 20. Om jeg fjerner // foran de to printf-linjene i koden over får jeg Tango x2, om jeg bare bruker den i main får jeg tom linje og cInfo endrer seg ikke uavhengig av hva jeg gjør. Skal ikke c->name nå peke mot Tango? Må jeg initialisere c->name på et annet vis?

 

Hele koden;

 

 

[spoiler]#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAX_AGE 200
#define MAX_LENGTH_NAME 30

typedef struct
{
int age;
char *name;
} pers;

//Constructor for struct person, set default values
const pers PERS_DEFAULT = {
-1, "-"
};

//Creates pers with default values
pers* pers_construct() {
pers *a = malloc(sizeof(pers));
*a = PERS_DEFAULT;
return a;
}

char* pers_getName(pers *p)
{
return p->name;
}

void pers_setName(pers *p, char *n)
{
if(strlen(n) > MAX_LENGTH_NAME) {
printf("Length is over 30 characters for name:\n%s\n", n);
printf("Name is not set.\n");
}
p->name = n;
}

int pers_getAge(pers *p)
{
return p->age;
}

void pers_setAge(pers *p, int a)
{
if(a > MAX_AGE) {
printf("Age is over 200 years. Age is not set.");
}
p->age = a;
}

int pers_checkSetName(pers *p)
{

if(strcmp(p->name, "-")) {
return 1;
}

return 0;
}

int pers_checkSetAge(pers *p)
{

if(p->age > -1) {
return 1;

}
return 0;
}

/* Concates name and age to an info-string
* Returns errormessage if one or both is not set
*/
char* pers_getInfo(pers *p)
{
char *returVal = malloc(sizeof(p->age) + sizeof(p->name));

if((pers_checkSetName(p)) && (pers_checkSetAge(p))) {
sprintf(returVal, "%s:%d", pers_getName(p), pers_getAge(p));
} else {
sprintf(returVal, "Name and/or age not set");
}

return returVal;
}

pers* pers_setInfo(char *setInfo)
{
pers *a = pers_construct(); //Creates pers-object

char line[1 + strlen(setInfo)]; //Creates char-array
strcpy(line, setInfo); //Copies pers-info into array

char *name = strtok(line, ":"); //Gets persname from passed info
char *age = strtok(NULL, ":"); //Gets persage from passed info

//Can't set variables if they're null
if(name == NULL || name == NULL) {
return NULL;
}

//Manual atoi, just for fun
unsigned int i;
int ageI = 0;
int mod = 1;

for(i = 0; i < strlen(age); i++) {
if(age[i] >= '0' && age[i] <= '9') {
ageI = ageI * 10 + age[i] - '0';
} else if(age[i] == '-' && i == 0) {
mod = -1;
}
}
ageI *= mod; //Modifier if age <0
//End manual atoi

pers_setName(a, name);
pers_setAge(a, ageI);
//printf("%s\n", a->name);
return a;
}

int main()
{
//Creates pers-objects as well as initialize c with customvalues
pers *a = pers_construct();
pers *b = pers_construct();
char *arba = "Tango:20";
pers *c = pers_setInfo(arba);

//printf("%s\n", c->name);
//Sets values for a and b
pers_setName(a, "Kalle");
pers_setName(b, "Klovn");
pers_setAge(a, 20);
pers_setAge(b, 21);

//Gets the personinfo
char *aInfo = pers_getInfo(a);
char *bInfo = pers_getInfo(b);
char *cInfo = pers_getInfo(c);
printf("%s\n", aInfo);
printf("%s\n", bInfo);
printf("%s\n", cInfo);

free(a);
free(b);
free(c);
free(aInfo);
free(bInfo);
free(cInfo);
return 0;
}

 

 

[Glutar]

Arrayen "line" i pers_setInfo er lokal og ligger på stacken. Når den går ut av scope (som den gjør når pers_setInfo" returnerer) kan minne den okkuperte brukes til andre ting. Du assigner a->name til dette minneområde i pers_setName.

 

Det c->name peker på er undefined.

 

Endre pers_setName til dette burde funke. (Dette er ikke en god løsning)

void pers_setName(pers *p, char *n)
{
    if(strlen(n) > MAX_LENGTH_NAME)
    {
        printf("Length is over 30 characters for name:\n%s\n", n);
        printf("Name is not set.\n");
    }
    p->name = malloc(strlen(n) + 1);
    strcpy(p->name, n);
}

Hvis du gjør dette må du huske å kalle free på c->name også.

Endret 9. september 2013 av Glutar

[Sleggefett]

Damn, noe slikt jeg frykta. Om jeg skriver a->name = name istedenfor i pers_setInfo, da vil vel det skape samme problem?

strcpy i pers_setName funka heller ikke..

 

Fiksa det sånn halvveis. Nå er malloc som du skrev flytta til pers_construct, og det er ingen kjøretidsfeil!

Kjedelige blir om jeg da må bruke free på alle navna.. Ingen annen mulighet for å frislippe alle variablene som jeg har allokert? Eventuelt andre måter å løse name-problematikken?

 

(Forøvrig brukte dem a->name istedenfor pers_setName i pers_setInfo, i fasiten. Skal vel forårsake samme problem?)

Endret 9. september 2013 av Sleggefett

[Glutar]

Hvis du definerer pers slik:

typedef struct
{
    int age;
    char name[MAX_LENGTH_NAME];
} pers;

Også istedenfor bruke strcpy til p->name så slipper du malloc/free på p->name.

 

så noe slik:

void pers_setName(pers *p, char *n)
{
    if(strlen(n) > MAX_LENGTH_NAME) {
        printf("Length is over 30 characters for name:\n%s\n", n);
        printf("Name is not set.\n");
        return;
    }
    strcpy(p->name, n);
}

[Sleggefett]

Aaah, så klart. Har lagt litt vel mye i det at pekere og arrays kan brukes ganske likt.

 

(og takk for hint om at jeg glemte return i setname og setage-metodene! )

[Lycantrophe]

Vit at det ikke nødvendigvis er pent, fordi det lekker intern struktur.

[Sleggefett]

Lekker intern struktur? Er ikke minneplassen allokert og satt når jeg kaller på pers_construct?

[Lycantrophe]

Det er ikke det jeg sikter til. Om noen av ordene jeg bruker her går HELT over hodet på deg kan du se bort i fra denne posten.

 

For en sak som dette er det uvesentlig. Men det er vanlig, når man designer biblioteker og lignende, å ikke eksponere structene sine direkte. På den måten har man muligheten til å forandre implementasjonen, modifisere felter, legge til og fjerne etc. uten at det forandrer bibliotekets interface.

 

Ved å bare forward declare

 

 

struct person;

 

i headeren din, og ved å la alle funksjoner ta en person*, altså en peker til structen, vil du fint kunne bytte ut ting senere. Ting vil fra utsiden, såfremt du ikke forandrer tilhørende funksjoner, naturligvis, se ut som det alltid ha gjort. Det er en måte å strukturere koden sin på, ved å la structs være som små, svarte bokser.

 

En annen fordel er at det gjør at brukerene dine (du kan selv være disse brukerene) ikke vil trenge å bli opphengt i og snuble i implementasjonen; de vil kun trenge å ta hensyn til funksjonen det skal ha.

[Sleggefett]

Hva legger du i "eksponere structene"? Hadde det vært java hadde jeg tenkt public/private med en gang, men vet ikke om det finnes slikt i C?

Husk at jeg er fortsatt ganske grønn, og den oppgaven er fra ukesoppgaver for uke2.

[Lycantrophe]

Det minner om public/private, ja. I tillegg hjelper deg deg å veldig tydelig skille interface fra implementasjon. C har ikke støtte for public/private direkte, men du kan fint emulere det ved å bruke denne teknikken. Opaque structs kalles den.

 

Se for deg en header:

 

exposed.h

struct exposed {
    int id;
    char shortname[ 10 ];
    ...
};

I annen kode kan jeg nå:

#include "exposed.h"
 
int main() {
    ...
    char name[ 10 ];
    memcpy( name, exposed.shortname, 10 );
}

Altså direkte tilgang til dataen. Se så for deg at vi har headeren opaque:

 

opaque.h:

struct opaque;
 
void read_name( opaque*, char* );

For ordens skyld ser vi bort ifra ting som array bounds og slikt nå. :--)

 

opaque.c - dette er filen hvor koden fra opaque.h faktisk blir implementert.

struct opaque {
    int id;
    char shortname[ 10 ];
};
 
void read_name( opaque* op, char* buffer ) {
    memcpy( buffer, op->shortname, 10 );
}

Og i annen kode:

#include "opaque.h"
 
int main() {
    ...
    char name[ 10 ];
    read_name( opaque_ptr, name );
}

Bonusrunde:

#1: Hvilke begrensninger setter dette på opaque?

#2: Hvorfor vil ikke eksempelkoden min fungere?

#3: Er det noen andre fordeler?

 

Svar:

#1: Mest innlysende er at opaque ikke lenger kan legges på stacken, fordi kompilatoren aldri får vite hvor stor den er. Skjønner du hva dette betyr?

#2: Fordi vi ikke har noen måte å instansiere opaque på. new gjør dette i Java. I C er det vanlig å tilby new_opaque og free_opaque-funksjoner.

#3: Mange! En innylsende er at det nå er umulig å gjøre dette:

memcpy( opaque->shortname, "garblegarblegarble", 42 );

Minner det deg om noe?

 

Jeg vet det er noen åpenbare svakheter i eksempelkoden, men det er mest fordi de vil være distraherende elementer når jeg skal illustrere selve teknikken. Om det er noe du ikke forstår eller lurer på er det selvfølgelig bare å spørre.

Endret 10. september 2013 av Lycantrophe

[Sleggefett]

Hmm, er det riktig da at header-filer både kan inneholde et rent interface og implementasjon? Nytt dette her med headerfiler, har ikke en gang rørt ved javaprogram med flere kildefiler.

 

Vel, #1 vil vel bety at vi ikke kan lage slike "objekter" som jeg gjorde i oppgaven her?

 

Likevel, ser absolutt at dette er noe å ta med seg videre. Takk skal du ha!

[Lycantrophe]

Ikke skriv funksjoner i headerfiler. Men feltene i en struct er fortsatt en implementasjon, og de sier ganske mye om hvordan alle funksjoner som bruker structen er satt sammen.

 

#1 betyr at du ikke lenger kan skrive:

 

 

opaque op;
printf( "%d\n", op.id );

 

fordi kompilatoren ikke vet hvor stor opaque er. I stedet må du gjøre noe á:

 

 

opaque* op = new_opaque();
printf( "%d\n", get_id( op ) );

 

Merk at dette ikke er svaret for alle situasjoner. På ingen måte. Men det er verdt å vite om konsekvensene ved å ikke gjøre det og.