C - создать строку из параметра структуры

Иметь

typedef struct person {
    char name[20]
    char surname[20]
} person_t;

Мне нужно создать строку типа XXXXXX:YYYYYY с функцией типа char* personToString(person_t *p). Я попытался сделать это:

char* personToString(person_t* p) {

  int n1,n2;
  n1=strlen(p->name);
  n2=strlen(p->surname);
  char *p = (char*) malloc((n1+n2+2)*sizeof(char));
  strcat(p,puser->name);
  strcat(p,":");
  strcat(p,puser->surname);

  return p;
}

Это дает мне разумный результат, но у меня есть некоторые ошибки при тестировании с помощью valgrind! Я также думаю, что есть более классный способ написать функцию!


person user2590319    schedule 18.07.2013    source источник
comment
Включите ошибки, поскольку они имеют отношение к вопросу.   -  person dandan78    schedule 18.07.2013
comment
вы можете использовать sprintf вместо strcat, например sprintf(p, "%s:%s", p->name, p->surname);, плюс вы переопределяете p, вы получаете его как параметр, а затем объявляете его как char *, и вы не free свой char *   -  person Opsenas    schedule 18.07.2013
comment
sizeof(char) не обязательно, так как всегда 1   -  person Yu Hao    schedule 18.07.2013
comment
Разве не должно быть struct person *p вместо person_t, поскольку person_t — это объект, а не typedef?   -  person mohit    schedule 18.07.2013
comment
я добавляю также char ':', поэтому я думаю, что мне нужно +2! Я пытался с +1 и выдает мне ошибки   -  person user2590319    schedule 18.07.2013
comment
@RedX +2 в порядке, он включает: также в последнюю строку   -  person Sanyam Goel    schedule 18.07.2013
comment
@SanyamGoel Вы правы. Я удалил свой комментарий.   -  person RedX    schedule 18.07.2013


Ответы (4)


Когда вы выделяете память для p, память будет содержать мусорные значения. Strcat добавит строку после нулевого символа, но в неинициализированной строке будет содержать случайные значения.

Замените первый strcat на strcpy.

person Community    schedule 18.07.2013

Тебе следует

strcpy(p,puser->name);

нет

strcat(p,puser->name);

malloc не инициализирует буфер нулем, поэтому strcat сначала ищет нулевой байт в p и, вероятно, не находит его, читая за конец буфера и, таким образом, аварийно завершает работу.

Вместо одного strcpy и двух strcat вы также можете написать один вызов sprintf:

sprintf(p, "%s:%s", puser->name, puser->surname);
person Werner Henze    schedule 18.07.2013
comment
вы правы :D большое спасибо! (у меня ошибка st**id) Даже если это работает хорошо, как вы думаете, есть ли более стильный способ написать код? - person user2590319; 18.07.2013
comment
Werner, Вы нас опередили, Ваш метод больше подходит для конкатенации более чем одной строки. + к вашему ответу. Хороший - person Grijesh Chauhan; 18.07.2013
comment
@ user2590319 Я бы использовал sprintf. Реализация VoidPointer мне кажется неплохой. - person Werner Henze; 18.07.2013

Сначала вы должны вызвать копирование строки, затем strcat:

strcat(p,puser->name);

должно быть:

strcpy(p,puser->name);

поскольку память, выделенная с помощью функции malloc, хранит значения мусора, выполняя сначала strcat, вы выполняете конкатенацию после мусора - это также приводит к неопределенному поведению в вашем коде.

Вы можете использовать void* calloc (size_t num, size_t size); вместо malloc(), функция calloc инициализирует выделенную память с помощью 0 ( тогда strcat() без проблем). Также при динамическом выделении памяти вы должны освободить блок памяти, явно используя void free (void* ptr);).

person Grijesh Chauhan    schedule 18.07.2013
comment
Чтобы добавить к ответу, Valgrind может жаловаться на то, что он не free обрабатывает память - person Suvarna Pattayil; 18.07.2013

Это выглядит хорошо для меня,

char* personToString( struct person_t *p )
{
  int len = strlen(p->name) + strlen(p->surname) + 2; // holds ':' + NULL 
  char *str = malloc( len ); // Never cast malloc's return value in C

  // Check str for NULL
  if( str == NULL )
  {
      // we are out of memory
      // handle errors
      return NULL;
  }

  snprintf( str, len, "%s:%s", p->name, p->surname);

  return str;
}

ПРИМЕЧАНИЕ:

  1. Никогда не переводите возвращаемое значение malloc в C.
  2. Используйте snprintf, когда нужно несколько strcat, это элегантно.
  3. free возвращаемое значение str здесь в вызывающей программе.

Фиксированные переменные struct и char.

person VoidPointer    schedule 18.07.2013
comment
Разве не должно быть struct person *p вместо person_t, поскольку person_t — это объект, а не typedef? - person mohit; 18.07.2013
comment
Хороший ответ, на самом деле у меня есть предложение изменить комментарий Never cast malloc's return value на Never cast malloc's return value in C - person Grijesh Chauhan; 18.07.2013
comment
Нет необходимости использовать snprintf, sprintf будет достаточно, так как вы убедились, что буфер достаточно большой. Кстати, это p, а не puser. И слишком много закрывающих скобок в строке snprintf. - person Werner Henze; 18.07.2013
comment
Проверка p на NULL, чем я могу установить значение errno? Например: if(p==NULL) { errno = EINVAL; return NULL; } это хорошо? - person user2590319; 18.07.2013
comment
@WernerHenze snprintf() здесь не нужен (потому что выделен правильно), но это предпочтительнее и безопаснее. - person Grijesh Chauhan; 18.07.2013
comment
@user2590319 user2590319 обновлен для обработки ошибки malloc. Что касается errno, см. stackoverflow.com/questions/11699596/how -to-set-errno-value - person VoidPointer; 18.07.2013