0

As far as I can tell I've followed the advice given by other users, but I still get a segfault when I try to return a pointer to an array. The printf in main returns nothing, so I'm guessing that the pointer isn't to anything on the heap?

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

int * stringy(int length){
   int *ptr = malloc(sizeof(int)*length);
   int i;
   for(i = 0; 1 < length; i++){
      ptr[i] = i;
   }
   return(ptr);
}

void main(int argc, char **argv){
   int strlen = 12;
   int *newptr = stringy(strlen);
   printf("%d\n",newptr[0]);
   free(newptr);
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Chib
  • 11
  • 4
  • 5
    `1 < length` -- are you certain you didn't mean `i < length`? –  Nov 26 '16 at 04:14
  • 2
    `for(i = 0; 1 < length; i++)`??? No wonder it segfaults. Spellchecking your code is something you are supposed to be able to do yourself, without requesting worldwide help. – AnT stands with Russia Nov 26 '16 at 04:14
  • 1
    @AnT Making fun of people is not very constructive. – Leonora Tindall Nov 26 '16 at 04:16
  • 2
    @SilverWingedSeraph: I think you are really stretching the concept of "making fun of people" beyond what's reasonable. Nobody is trying to "make fun" of anyone here. – AnT stands with Russia Nov 26 '16 at 04:18
  • @AnT An excessive number of questions marks along with exaggeration ('worldwide help') are really unnecessarily aggressive here, and are clearly meant to make this person feel bad. – Leonora Tindall Nov 26 '16 at 04:24
  • @SilverWingedSeraph: I admit that there is a certain level of educational sarcasm involved there. But still perceiving it as me "making fun" of the OP is completely unjustified. – AnT stands with Russia Nov 26 '16 at 04:28
  • @SilverWingedSeraph He's not making fun of me. He's pointing out that I screwed up that for loop... – Chib Nov 26 '16 at 04:31
  • @Chib, As long as you don't feel that it's too aggressive, then it's fine :) – Leonora Tindall Nov 26 '16 at 04:32
  • 2
    Using `strlen` as a variable name isn't precisely wrong, especially when you don't `#include ` and don't do any string manipulation, but it is not a good idea to use variable names that match any of functions defined in the standard library. If nothing else, it is likely to lead to confusion if the code is modified and does need to do string manipulation. – Jonathan Leffler Nov 26 '16 at 04:39
  • 1
    Also note that `void main()` is only allowed by Microsoft C; see [What should `main()` return in C and C++](http://stackoverflow.com/questions/204476/) for all the sordid details. – Jonathan Leffler Nov 26 '16 at 04:40
  • @SilverWingedSeraph That's really irrelevant. You don't go onto an internet forum and expect hugs and feel-good words when you ask for help writing code. I didn't spellcheck my code and he's telling me that I should spellcheck my code before posting a question on Stack Exchange. – Chib Nov 26 '16 at 04:43
  • Only on the Internet™: Someone becoming unhappy about me telling someone else to be nicer to them, after also answering their question. Anyway this is off topic, I'm going to stop responding. – Leonora Tindall Nov 26 '16 at 04:45
  • It is good to have practise that after freeing up the memory always point pointer to NULL(Just to avoid dangling pointer). `newptr=NULL` In your case, program is terminating after `free()`. So the resources of your code will get also freed. No need to worry here. But in case of program which is executing continuously, there you will have to take care of dangling pointer. – Akshay Patole Nov 26 '16 at 05:37

2 Answers2

6

It seems like there is a typo in your code. Instead of

for(i = 0; 1 < length; i++){

, it looks like you meant

for(i = 0; i < length; i++){
John Bollinger
  • 160,171
  • 8
  • 81
  • 157
Jarvis
  • 8,494
  • 3
  • 27
  • 58
  • 2
    Do NOT cast the return of `malloc`. It is pointless. See: [**Do I cast the result of malloc?**](http://stackoverflow.com/q/605845/995714) for thorough explanation. – David C. Rankin Nov 26 '16 at 04:25
  • @Jarvis: You should not typecast malloc return value. Please read this: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – MayurK Nov 26 '16 at 04:27
  • Yeah, I know that in `C`, `void *` is automatically and safely promoted to any other pointer type, but in order to compile it using `g++`, it's necessary to type-cast, and that's why I included this in my answer. But you read the answer of `Ron Burk` to the question `Do I cast the result of malloc?`, I think it makes more sense. – Jarvis Nov 26 '16 at 04:36
  • 1
    The question is tagged C, not C++. These are different languages, as reflected in part by the fact that some good, idiomatic C code is invalid in C++. That is not a basis for rejecting good coding practices, such as avoiding casting the return value of `malloc()`. Use g++ only to compile C++ code. – John Bollinger Nov 26 '16 at 05:41
  • Yes, you are right but I was just trying to share some extra info, and there is no harm in that. There was no need to downvote this answer, as long as my answer is answering the OP's question. @JohnBollinger – Jarvis Nov 26 '16 at 05:43
  • 2
    Giving bad advice along with the good makes this a bad answer. A worse answer, in some ways, than if the whole thing were bad. – John Bollinger Nov 26 '16 at 05:49
  • Can you make edits to this answer, and hopefully, then up-vote too ? :) @JohnBollinger – Jarvis Nov 26 '16 at 05:56
  • @Jarvis, I have edited the answer per your request, and lifted my downvote. The remaining part of the answer is correct and useful, but it also underscores the fact that the question should have been closed, not answered. With luck it may still be closed. – John Bollinger Nov 26 '16 at 06:08
  • if you get errors about the cast then you're compiling it in C++ mode – phuclv Nov 26 '16 at 06:21
1

You need to check for null pointers; malloc returns NULL when it can't allocate the memory you requested, and that would cause the problem you're referring to. Also, you should cast your pointer to int *.

The actual problem, however, is that you have a typo; rather than 1 < length, your for loop condition should read i < length. Your for loop runs forever.

Leonora Tindall
  • 1,391
  • 2
  • 12
  • 30
  • I ditched that part of the code, just to see whether it would get rid of the error. The for loop declaration was the issue though. – Chib Nov 26 '16 at 04:34
  • You should *not* cast the return value of `malloc()` in C, unless you are making the extraordinary effort required to code simultaneously in C and C++. And that's tricky to do, and rarely worthwhile. – John Bollinger Nov 26 '16 at 05:46