Segmentation fault

[LL]

#include <stdio.h>

float* dup(float* farr[], int n) {
float* fdarr;
for (int i=0; i<n; i++) {
fdarr=*(farr);
}
return fdarr;
}

main() {
float *f1=new float(1.0);
float *f2=new float(2.0);
float *f3=new float(3.0);
float* farr_s[]={f1,f2,f3};

float* fdarr_s;
fdarr_s=dup(farr_s,3);

for (int i; i<3; i++) {
printf("%f ", fdarr_s); // Segmentation fault
}
}

 

[Roberto Divia]

#include <stdio.h>

float* dup(float* farr[], int n) {
float* fdarr;

You did not allocate any memory for fdarr...

for (int i=0; i<n; i++) {
fdarr=*(farr);
}
return fdarr;

You return something which points to the void.

}

main() {
float *f1=new float(1.0);
float *f2=new float(2.0);
float *f3=new float(3.0);
float* farr_s[]={f1,f2,f3};

float* fdarr_s;
fdarr_s=dup(farr_s,3);

for (int i; i<3; i++) {
printf("%f ", fdarr_s); // Segmentation fault
}
}

I think that "gcc -Wall" will give you the right warning at compile time.

Ciao,
--
Roberto Divia` Love at first sight is one of the greatest
DepH Bat:53 Mailbox:C02110 labour-saving devices the world has ever seen
Route de Meyrin 385 ---------------------------------------------
Case Postale Phone: +41-22-767-4994
CH-1211 Geneve 23 CERN Fax: +41-22-767-9585
Switzerland E-Mail: (e-mail address removed)

 

[Kojak]

Le 02 Mar 2009 06:59:09 GMT,
LL a écrit :

#include <stdio.h>
[...]

Here is a slightly modified version of your code, it should
breathe much better now.

........8<.......8<.......8<.......8<.......8<.......8<.......8<
#include <stdio.h>
#include <stdlib.h>
#include <malloc.h>

void dup (float *fdarr, float *farr)
{
int n = sizeof(farr) / sizeof(float) + 1;

while (n--)
*fdarr++ = *farr++;
}


int main (void)
{
float f1 = 1.0;
float f2 = 2.0;
float f3 = 3.0;

float farr_s[] = {f1, f2, f3};

float *fdarr_s;

fdarr_s = malloc(sizeof(farr_s));

dup(fdarr_s, farr_s);

for (int i=0; i<=2; i++)
printf("%f ", fdarr_s);

printf("\n");

return EXIT_SUCCESS;
}
........8<.......8<.......8<.......8<.......8<.......8<.......8<

 

[Ike Naar]

Here is a slightly modified version of your code, it should
breathe much better now.

void dup (float *fdarr, float *farr)
{
int n = sizeof(farr) / sizeof(float) + 1;

while (n--)
*fdarr++ = *farr++;
}

Inside dup(), farr is a pointer to float, not an array of float.
So sizeof(farr) yields the same value as sizeof(float*).

 

[Guest]

#include <stdio.h>

float* dup(float* farr[], int n) {
  float* fdarr;
  for (int i=0; i<n; i++) {
    fdarr=*(farr);
  }
  return fdarr;

}

main() {
  float *f1=new float(1.0);
  float *f2=new float(2.0);
  float *f3=new float(3.0);
  float* farr_s[]={f1,f2,f3};

  float* fdarr_s;
  fdarr_s=dup(farr_s,3);

  for (int i; i<3; i++) {
    printf("%f ", fdarr_s); // Segmentation fault
  }

Idiot. You are *still* posting C++ to comp.lang.c.
Don't Do This.

 

[Kojak]

Le Mon, 02 Mar 2009 09:29:39 +0000,
Ike Naar a écrit :

Inside dup(), farr is a pointer to float, not an array of float.
So sizeof(farr) yields the same value as sizeof(float*).

Oops! Absolutely right. Always check before sending...

Hopefully, the right now:

........8<.......8<.......8<.......8<.......8<.......8<.......8<
#include <stdio.h>
#include <stdlib.h>
#include <malloc.h>

void dup (float *fdarr, float *farr, int n)
{
while (n--)
*fdarr++ = *farr++;
}


int main (void)
{
float f1 = 1.0;
float f2 = 2.0;
float f3 = 3.0;
float farr_s[] = {f1, f2, f3};
float *fdarr_s;
int n = sizeof(farr_s)/sizeof(float);

fdarr_s = malloc(sizeof(farr_s));

dup(fdarr_s, farr_s, n);

for (int i=0; i<n; i++)
printf("%f ", fdarr_s);

printf("\n");

return EXIT_SUCCESS;
}
........8<.......8<.......8<.......8<.......8<.......8<.......8<

 

[LL]

#include <stdio.h>

float* dup(float* farr[], int n) {
float* fdarr;
for (int i=0; i<n; i++) {
fdarr=*(farr);
}
return fdarr;
}

main() {
float *f1=new float(1.0);
float *f2=new float(2.0);
float *f3=new float(3.0);
float* farr_s[]={f1,f2,f3};

float* fdarr_s;
fdarr_s=dup(farr_s,3);

for (int i; i<3; i++) {
printf("%f ", fdarr_s); // Segmentation fault
}
}

Sorry for posting in a wrong forum. But I fixed it anyways.

#include <stdio.h>

float* dup(float* farr[], int n) {
float* fdarr=new float[n];
for (int i=0; i<n; i++) {
fdarr=*(farr);
}
return fdarr;
}

main() {
float *f1=new float(1.0);
float *f2=new float(2.0);
float *f3=new float(3.0);
float* farr_s[]={f1,f2,f3};

float* fdarr_s=dup(farr_s,3);

for (int i=0; i<3; i++) {
printf("%f ", fdarr_s); // 1.0 2.0 3.0
}
}

 

[Mark Wooding]

Here is a slightly modified version of your code, it should
breathe much better now.

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


void dup (float *fdarr, float *farr)

Calling a function `dup' is a bad idea -- especially if you give it
external linkage. The name `dup' is a well-known and useful function
defined by POSIX; your program risks not being portable to POSIX systems
if you define such names yourself.

{
int n = sizeof(farr) / sizeof(float) + 1;

No. No, no, no. sizeof(farr) is a compile-time constant, being the
number of chars occupied by an object of type pointer-to-float. It has
nothing to do with the size of any array which might happened to be
passed into this function (indirectly, as a pointer to the first
element).

while (n--)
*fdarr++ = *farr++;

Even if sizeof(farr)/sizeof(float) happened to be the number of elements
in the array indicated by farr, n would be one too large. The above
loop will execute n times precisely (`while (--n)' would execute n - 1
times). This is therefore a buffer overflow bug, and a potential
security vulnerability. (Oh, and it's undefined behaviour.)

}


int main (void)
{
float f1 = 1.0;
float f2 = 2.0;
float f3 = 3.0;

float farr_s[] = {f1, f2, f3};

float *fdarr_s;

fdarr_s = malloc(sizeof(farr_s));

You don't check the return from malloc. If fdarr_s is null, you'll get
undefined behaviour in dup (called below).

dup(fdarr_s, farr_s);

As discussed above, you really ought to pass in the number of elements.
Of course, by the time you've done that, you're probably better off
using memcpy anyway.

for (int i=0; i<=2; i++)

Style: it's probably better to use

for (int i = 0; i < 3; i++)

since this shows the size of the array and the number of iterations
directly rather than being off-by-one.

printf("%f ", fdarr_s);

printf("\n");

return EXIT_SUCCESS;
}

-- [mdw]

 

[Richard Bos]

Sorry for posting in a wrong forum. But I fixed it anyways.

No, you didn't.

#include <stdio.h>

float* dup(float* farr[], int n) {
float* fdarr=new float[n];

See? It's still broken.

Richard

 

[Kojak]

Le Mon, 02 Mar 2009 14:33:56 +0000,
Mark Wooding a écrit :

<malloc.h> is a system-specific header. Its use is nonportable. I've
no idea why you've seen fit to include it here.

Indeed, it's pointless here, just an old and bad reflex.

void dup (float *fdarr, float *farr)

Calling a function `dup' is a bad idea [...]

I know, but it's the OP choice, not mine.


Just a sort of brain typo, already corrected (but, I'm sure, you
already noticed).

You don't check the return from malloc. If fdarr_s is null, you'll
get undefined behaviour in dup (called below).

It's up to the OP to do the job.

That is, thank you for your time lost in corrections even if
I'm not concerned about them, the OP will appreciate.

As already said, IMHO, it's up to the OP to do analyse and
required corrections, modifications, adaptations and what else he
need, not me. My time is limited, then I try to reply as quickly
and as accuracy as I can, no time to test, check, and think about
all little details. I'm not paid for that (my job do) nor something
to prove or exams to validate.

You would have been wiser to reply the OP, and, at the same
time, to give your own version of the code.

That said, if you have time to wast then, instead of correcting
code, please correct my English, I would better appreciate.
Really.

Cheers,