Pointer als Rückgabewert einer Funktion

(Wissensstand Anfang 2 Semester Medieninformatik)

Hallo, ich habe Folgendes Problem:
ich soll eine Aufgabe bearbeiten, die Folgendermaßen lautet:
"Die Funktion getCoordinateArray() soll auf dem Heap ein Feld zur Speicherung von Koordinaten (Datentyp coordinate_t) erzeugen und einen Zeiger auf das erste Feldelement zurückliefern. Die Feldgröße n wird beim Aufruf mitgegeben. Die x- und y-Koordinaten der Feldelemente sollen mit 0 initialisiert werden."

Ich habe in der Funktion die Feldelemente mit 0 initialisiert, und dieser den Rückgabewert des pointers gegeben.
Wenn ich das programm jetzt ausführe, erhalte ich zwar häufig den Wert 0.00000
allerdings werden mir zwischendrin komische Werte ausgegeben, die dort eigentlich nicht stehen sollten.

Kann mir jemand helfen, den Fehler im Code zu finden?
Ich bin mit meinem Latein am Ende, danke schon mal im Voraus!
[code="c"]#include<stdio.h>
#include<stdlib.h>

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

coordinate_t * getCoordinateArray (int n)
{
int a, b;
coordinate_t CoordArray[n];
coordinate_t* point;
point = CoordArray;

for(a = 0; a < n; a++)
{
for(b = 0; b < 1; b++)
{
CoordArray[n].x = 0;
CoordArray[n].y = 0;
}
}

return point;
}

int main ( void)
{
coordinate_t * q;
int i = 0;
q = getCoordinateArray(10);
for ( i=0; i < 10; i++)
printf("(%f,\n%f)\n", q[i].x, q[i].y);
free(q);
return 0;
}

Wutz

- gewöhne dir an, deinen Code vernünftig einzurücken
- gewöhne dir an, Codetags zu benutzen
- VLA benutzen nur Deppen
- du sollst den Speicher auf dem Heap anlegen, nicht auf dem Stack (so wie du es mit dem Deppen-VLA gemacht hast)
- dein printf arbeitet auf ungültigem Speicher
- dein free arbeitet auf ungültigem Speicher (sollte eigentlich abbrechen)

Der Code ist bei mir eingerückt, da habe ich wohl was mit der Eingabe hier im Forum falsch gemacht.
Die Aufgabe war es, nur den Inhalt der Funktion nachzutragen, der Rest wurde vom Prof vorgegeben.

Mal als Gegenfrage: Was ist an meiner Funktion alles falsch und was nicht?

Grüße

Haloelite2 schrieb:

Mal als Gegenfrage: Was ist an meiner Funktion alles falsch und was nicht

Du legst nix auf dem Heap an. Dann stünde irgendwo (*alloc).
Darüberhinaus legt Du ein lokales (Variable Length) Array (VLA) an und gibst dessen Adresse nach aussen.
Die verschachtelten for Schleifen sind auch obskur - wohl ein Überbleibsel?!
Ein Schnitzer ist allerdings, dass Du weder a noch b verwendest (Du weist immer an der Stelle [n] zu - die liegt zu allem Überfluss außerhalb des Arrays...).
"Korrekter" wäre evtl. auch 0.f zuzuweisen - also ein float statt 0 (ein int ) - Aber das ist schon fast kleinlich.

Du siehst schon: es stimmt nicht viel mit der Funktion...Aber das ist halt so, wenn man Sachen lernt.

PS:

#define getCoordinateArray(x) calloc((x),sizeof(coordinate_t))

SCNR...

Belli

Deine Funktion erstellt das Feld auf dem Stack und nicht auf dem Heap. Der Stack wird aber bei Funktionsende wieder abgeräumt, Dein Feld existiert dann nicht mehr. Der Pointer, den Du zurückgibst, zeigt deshalb auf ungültigen Speicherbereich.

Um Speicher auf dem Heap zu reservieren, benutze die Funktion malloc.

Okay, ich überarbeite noch ein mal alles und poste die Antwort dann später noch mal, in der Hoffnung, das mehr richtig ist.

Danke.

Also, ich habe meinen Code verbessert.
Die einzige Frage, die mir bleibt ist, ob ich die Feldwerten in der Funktion auf Null setzen muss, oder ob es reicht, wenn ich sie einfach nicht deklariere.
Naja, hier mein Code:

#include<stdio.h>
#include<stdlib.h>
//#define getCoordinateArray(x) calloc((x),sizeof(coordinate_t))

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

coordinate_t * getCoordinateArray (int n)
        {coordinate_t* point;
        point = (int*)malloc(sizeof(coordinate_t));
        coordinate_t Feld[n];

        return point;
        }

int main ( void)
{
    coordinate_t * q;
    int i = 0;
    q = getCoordinateArray(10);
    for ( i=0; i < 10; i++)
        printf("(%f,\n%f)\n", q[i].x, q[i].y);
        free(q);
    return 0;
}

Die Funktion sollte so aussehen

coordinate_t * getCoordinateArray (int n)
        {
        return (coordinate_t*)calloc(sizeof(coordinate_t) * n); // Wichtig: * n und (coordinate_t*)
        }

calloc füllt das feld mit 0 (ist aber langsamer)

Für wieviele Elemente vom Typ coordinate_t legst Du in Z.14 Speicherplatz an?
Welche Überlegung steht hinter dem cast in Z.14? (int*)
Welche Überlegung steht hinter dem VLA Feld in Z. 15?

Schalt die Warnungen in Deiner IDE ein! Der Compiler sieht viel mehr Merkwürdigkeiten als Du!

Und so wie ich es gemacht habe, ist es nicht richtig?
wenn ich mein Programm ausführe, bekomme ich auch 0.0000 als Feldwert zurück.

Stimmt, das (int*) ist natürlich vollkommen unsinnig!
Klar, das Feld brauche ich gar nicht, da in main() eines angelegt ist.

Was ich nicht verstehe ist das:

calloc(sizeof(coordinate_t) * n); // Wichtig: * n und (coordinate_t*)

Wieso das *n?
Und kann man den cast vor malloc nicht weg lassen? (Oder ist das bei calloc anders geregelt?)

Danke.

Belli

Du hast auf dem Heap aber nur Speicher für eine einzige struct reserviert. Außerdem ist die Definition von Feld sinnlos.
Der cast des Rückgabewertes von malloc ist 1. überflüssig und 2. in Deinem Fall auch noch falsch.

Belli

Naja, sizeof(coordinate_t) ist die Größe eines structs. Du willst aber Platz für n structs, deshalb:

sizeof(coordinate_t) * n

// in Worten: mal n

Belli

Haloelite2 schrieb:

Klar, das Feld brauche ich gar nicht, da in main() eines angelegt ist.

Nein. Das Feld brauchst Du gar nicht, weil Du es einfach nicht brauchst ...
Wo ist denn in main eines angelegt?

Edit:
Ach ja, natürlich solltest Du - wie in der Aufgabe verlangt - die Feldelemente in Deiner Funktion auch (mit 0) initialisieren!

In Ordnung, dann mal vielen Dank, an alle die geholfen haben.
ich denke, ich habe es wenigstens vom Prinzip her verstanden.

for ( i=0; i < 10; i++)
printf("(%f,%f)\n", q[i].x, q[i].y);

Stimmt, mit q[i].x, bzw. q[i].y sind ja Speicherstellen (soweit ich weiß) gemeint.
Schon peinlich.

coder777 schrieb:

return (coordinate_t*)calloc(sizeof(coordinate_t) * n); // Wichtig: * n und (coordinate_t*)

Das ist falsch!#
calloc erwartet zwei Paramter
malloc erwartet einen Paramter

http://www.cplusplus.com/reference/cstdlib/calloc/
http://www.cplusplus.com/reference/cstdlib/malloc/

Schreib Dir eine Funktion für das "initialisieren" solcher Felder und eine für die Ausgabe.

// Weist den ersten n coordinate_t Objekten im Array base die Werte x und y zu
void coordinate_init(coordinate_t *base, int n, float x, float y);

// Gibt die ersten n coordinate_t Objekte im Array base nach stdout aus.
void coordinate_print(const coordinate *base, int n);

Vergiss nicht den Rückgabewert von getCoordinateArray() in main() zu testen. Eventuell konnte nicht genügend Speicher alloziert werden und malloc() gibt NULL zurück.
Überleg Dir auch, zu überprüfen ob der Parameter n in irgendeiner der Funktionen negativ ist - das wäre wohl ein Fehler vom Aufrufer.

Also in meinem Fall malloc benutzen?

Haloelite2 schrieb:

Also in meinem Fall malloc benutzen?

Ich habe nur an den Lerneffekt gedacht.

Die calloc() Lösung funktioniert übrigens nur, wenn die 0 als float wirklich kein Bit gesetzt hat. Das ist wohl bei so ziemlich jedem System der Fall - aber 100% ist es nicht.
Du kannst das natürlich testen...blabla... Aber warum?! Nimm malloc() und initialisier selber.

#if __STDC_IEC_559__==1
#define getCoordinateArray(x) calloc((x),sizeof(coordinate_t))
#else
coordinate_t *getCoordinateArray(int n) {
  coordinate_t *result = malloc(n*sizeof(coordinate_t));
  if(result)
    coordinate_init(result, n, 0.f, 0.f);
  return result;
}
#endif