Это правильный способ вернуть массив структур из функции? - PullRequest
Новая
       19

Это правильный способ вернуть массив структур из функции?

5 голосов
/ 19 мая 2011

Как видно из названия (и подсказывает), я новичок в C и пытаюсь вернуть массив функций произвольного размера из функции.Я решил использовать malloc, так как кто-то в Интернете, чей умнее меня, указал, что если я не выделю кучу, массив будет уничтожен, когда points_on_circle завершит выполнение, и будет возвращен бесполезный указатель.

Код, который я представляю, работал, но теперь я все больше и больше вызываю функцию в своем коде, получаю ошибку времени выполнения ./main: free(): invalid next size (normal): 0x0a00e380.Я предполагаю, что это связано с моей взломанной реализацией массивов / указателей.

Я пока не вызываю free, так как многие из создаваемых мной массивов должны будут сохраняться повсюдужизнь программы (я добавлю добавление free() вызовов к остатку!). 

xy* points_on_circle(int amount, float radius)
{
  xy* array = malloc(sizeof(xy) * amount);

  float space = (PI * 2) / amount;

  while (amount-- >= 0) {
    float theta = space * amount;

    array[amount].x = sin(theta) * radius;
    array[amount].y = cos(theta) * radius;
  }

  return array;
}

Моя новаторская структура xy определяется следующим образом: 

typedef struct { float x; float y; } xy;

И пример того, как я вызываю функцию, выглядит следующим образом: 

xy * outer_points = points_on_circle(360, 5.0);
for(;i<360;i++) {
    //outer_points[i].x
    //outer_points[i].y
}

A указатель в правильном направлении.

Ответы [ 7 ]

7 голосов
/ 19 мая 2011

Выделение памяти в одной функции и освобождение ее в другой чревато опасностью.

Я бы выделил память и передал ее (буфер памяти) функции с параметром, указывающим, сколько структур разрешенозаписать в буфер.

Я видел API, в которых есть две функции: одна для получения необходимой памяти, а другая - для фактического получения данных после выделения памяти.

[Изменить] Нашел пример: http://msdn.microsoft.com/en-us/library/ms647005%28VS.85%29.aspx

6 голосов
/ 19 мая 2011

Я бы сказал, что этот дизайн программы в корне неверен.Во-первых, логически функция, которая выполняет вычисления, не имеет ничего общего с распределением памяти, это две разные вещи.Во-вторых, если только функция, выделяющая память, и функция, освобождающая ее, не принадлежат одному и тому же программному модулю, дизайн будет плохим, и вы, скорее всего, получите утечки памяти.Вместо этого оставьте выделение вызывающей стороне.

Код также содержит различные опасные действия.Избегайте использования операторов - и ++ как части сложных выражений, это очень распространенная причина ошибок.Ваш код выглядит так, как будто он содержит фатальную ошибку и записывает границы массива только потому, что вы смешиваете - с другими операторами.В языке C нет никаких причин делать это, так что не делайте этого.

Еще одна опасная практика - это зависимость от неявных преобразований типов языка C от целых до float (балансировка, иначе говоря) от обычных арифметических преобразований.«).Что такое "PI" в этом коде?Это int, float или double?Исход кода будет зависеть от этого.

Вот что я предлагаю вместо этого (не проверено): 

void get_points_on_circle (xy* buffer, size_t items, float radius)
{
  float space = (PI * 2.0f) / items;
  float theta;
  signed int i;

  for(i=items-1; i>=0; i--)
  {
    theta = space * i;

    buffer[i].x = sin(theta) * radius;
    buffer[i].y = cos(theta) * radius;
  }
}
6 голосов
/ 19 мая 2011

РЕДАКТИРОВАТЬ: Вы возвращаете массив правильно , но ...

Предположим, вы создаете массив с 1 элементом 

xy *outer_points = points_on_circle(1, 5.0);

Что происходит внутри функции?
Давайте проверим ... 

  xy* array = malloc(sizeof(xy) * amount);

выделить место для элемента 1. OK! 

  while (amount-- >= 0) {

1 больше или равно 0, поэтому цикл выполняется (и количество уменьшается)
после установки array[0] вы вернетесь к началу цикла 

  while (amount-- >= 0) {

0 больше или равно 0, поэтому цикл выполняется (и количество уменьшается)
Теперь вы пытаетесь установить array[-1], что недопустимо, поскольку индекс относится к области вне массива.

3 голосов
/ 19 мая 2011

Я думаю, что это: 

while (amount-- >= 0 ) {

должно быть: 

while ( --amount >= 0 ) {

Рассмотрим случай, когда сумма изначально равна нулю.

2 голосов
/ 19 мая 2011

Насколько я понимаю, вы поступаете правильно, если, конечно, ваши абоненты бесплатно получают результаты.

Некоторые люди предпочитают распределять память и освобождать ответственность в одном и том же месте (для симметрии), то есть вне вашей функции. В этом случае вы должны передать предварительно выделенный xy* в качестве параметра и вернуть размер буфера, если указатель был нулевым: 

int requiredSpace = points_on_circle(10, 10, NULL);
xy* myArray = (xy*)malloc(requiredSpace);
points_on_circle(10, 10, myArray);
free(myArray);
1 голос
/ 19 мая 2011

Хотя ваше решение отсчитать количество и использовать его вместо того, чтобы использовать временное значение, сэкономило несколько циклов памяти, оно более концептуально запутанно. Особенно в случае, когда математика занимает здесь все время, вы сохраняете только дроби.

Но это не причина, по которой вы должны тратить незначительное количество времени. Этот вопрос является причиной: вы потратили часы! Это касается даже самых опытных и умных программистов. Ошибки просто более сложны и выходят за рамки ответа от stackoverflow! Другими словами, принцип Питера применим и к кодированию.

Не совершайте ошибку, приобретая опыт, который поможет вам избежать рисков такого рода, чтобы сэкономить цикл или два. Вот почему Макконнелл в Code Complete перечисляет Humility как положительный атрибут программиста.

Вот решение, с которого вы, вероятно, подумали начать: 

xy* points_on_circle(int amount, float radius)
{
  xy* array = malloc(sizeof(xy) * amount);

  float space = (PI * 2) / amount;

  int index;

  for (index=0;index<amount;index++) {
    float theta = space * index;

    array[index].x = sin(theta) * radius;
    array[index].y = cos(theta) * radius;
  }

  return array;
}

Если вам нужна скорость, вы можете сделать крошечную вещь, поставив тэту за пределы цикла, равной 0, и добавлять «пробел» каждый раз, так как + обязательно будет дешевле, чем * в плавающей запятой.

ускорить его в 10 раз или больше?

Если вам нужна серьезная скорость, этот совет от answers.com даст вам улучшение в 10 раз, если вы все сделаете правильно:

По теореме Пифагорама x2 + y2 Радиус2. Тогда просто решить для х или у, учитывая другой вместе с радиус. Вам также не нужно вычислить для всего круга - вы можете вычислить для одного квадранта и генерировать остальные три квадранта по симметрии. Вы поколение цикл будет, для Например, просто итерация от источника до радиус как х дельта х, порождая у, и отражая это в трех других квадрант. Вы также можете рассчитать для одна половина квадранта, и использовать оба симметрия и отражение для генерации остальные семь с половиной квадрантов.

Когда мне было 12 лет, я думал, что мне нравится рисовать кружок, используя sin и cos в графике 8 на моем atari 800. Мой двоюродный брат Марти (в последнее время работавший с робототехникой Microsoft) стер мою программу и реализовал вышеуказанное решение, используя только сложение в цикле, если я правильно помню, и нарисуйте тот же круг за 4 секунды вместо минуты! Если бы я не был крещен, я бы поклонился в поклонении. Жаль, что у меня нет удобного кода, но я бы поспорил, что немного погуглив его. Кто-нибудь?

1 голос
/ 19 мая 2011

Вы перебираете один элемент для большого.

Вы должны использовать вместо этого оператор , возвращающийся к нулю : 

while ( amount --> 0) {
    ...
}

Выделение памяти в одной функции иОсвободить его в другом - это все равно, что дать пистолет четырехлетнему ребенку.Ты не должен этого делать.

...