Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #7013 > unrolled thread

a little c programming that no works!!

Back to article view | Back to comp.lang.c

Contents

  a little c programming that no works!! Vinicio Flores <vfloreshdz@gmail.com> - 2011-06-26 20:50 -0700
    Re: a little c programming that no works!! Ian Collins <ian-news@hotmail.com> - 2011-06-27 16:07 +1200
      Re: a little c programming that no works!! Dirk Zabel <zabel@riccius-sohn.eu> - 2011-06-27 11:37 +0200
        Re: a little c programming that no works!! Ian Collins <ian-news@hotmail.com> - 2011-06-27 22:44 +1200
    Re: a little c programming that no works!! "BartC" <bc@freeuk.com> - 2011-06-27 09:44 +0100
    Re: a little c programming that no works!! Barry Schwarz <schwarzb@dqel.com> - 2011-06-27 06:04 -0700
    Re: a little c programming that no works!! "Heinrich Wolf" <invalid@invalid.invalid> - 2011-06-27 23:54 +0200
      Re: a little c programming that no works!! Lew Pitcher <lpitcher@teksavvy.com> - 2011-06-27 18:11 -0400
      Re: a little c programming that no works!! Barry Schwarz <schwarzb@dqel.com> - 2011-06-27 15:44 -0700
      Re: a little c programming that no works!! Keith Thompson <kst-u@mib.org> - 2011-06-28 22:40 -0700
      Re: a little c programming that no works!! "Heinrich Wolf" <invalid@invalid.invalid> - 2011-06-29 15:44 +0200
        Re: a little c programming that no works!! Keith Thompson <kst-u@mib.org> - 2011-06-29 10:06 -0700

#7013 — a little c programming that no works!!

Hello C hackers, this program that I am writing print "Segmentation
fault" when I execute it, I think that the problem is the gets
function,  but I am not sure

/** A simple database system **/

#include "datbase.h"

static int search_and_open_data_base(char *Arg)
{
  FILE *f;

  f = fopen(Arg, "r");

  if(!f)
    return 1;
  else
    return 0;
}

/* ************************************************ */

static FILE *create_database(char *arg)
{
  FILE *newbase;

  newbase = fopen(arg, "w+");

  return newbase;
}

/* ************************************************ */

static int print_options()
{
  clrscr();
  setattr(REVERSE);
  setcolor(FOREBLUE);

  gotoxy(2, 2);
  cprintf("DatBASE");

  gotoxy(2, 4);
  setattr(NORMAL);
  setcolor(FOREGREEN);
  cprintf("I)Insert new register  R)Print a register S) Exit");

  gotoxy(2, 6);
  setcolor(FORERED);
  cprintf("Option: ");

  setattr(NORMAL);
  normvideo();

}

/* ************************************************ */

static int open_and_fill_database(char *arg)
{
  FILE *database;

  struct datbase_registers *base = malloc ( sizeof(struct registers
*));

  database = fopen(arg, "ra");

  gotoy(10);
  puts("Insert register name: ");
  gets(base->regname);

  gotoy(11);
  puts("Insert name: ");
  gets(base->name);

  gotoy(12);
  puts("Insert telephone number: ");
  scanf("%ld", base->ntel);

  gotoy(13);
  puts("Insert age: ");
  scanf("%ld", &base->age);

  fprintf(database, "Register name: ");
  fputs(base->regname, database);

  fprintf(database, "\n\nPerson's name: ");
  fputs(base->name, database);

  fprintf(database, "\nTelephone number: %lf\n", base->ntel);

  fclose(database);
}

/* ************************************************ */

static int get_and_process_input()
{
  int ch;

  ch = getch();

  switch(ch)
    {
    case 'i':
    case 'I':
      open_and_fill_database("data.base");
      break;
    case 'r':
    case 'R':
      break;
    case 's':
    case 'S':
      setattr(NORMAL);
      normvideo();
      reset();
      ret;
      break;
    default:
      gotoy(9);
      cprintf("?Unknown operation\n");
      sleep(1);
      print_options();
      get_and_process_input();
      break;
    }
}

/* ********************|*************************** */
/* --------------------|--------------------------- */
/* ************| MAIN FUNCTION |******************* */

int main()
{
  FILE *database;

  int c;
  int search = search_and_open_data_base("data.base");


  if(search == 1)
    create_database("data.base");

  else
    {
      print_options();
      get_and_process_input();
    }
  return 0;
}


Here is the datbase.h  C header:
/** A simple database system **/

#include <ttk.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

#define MAX    ((900*800)*256)

struct datbase_registers
{
  char regname[MAX];
  char name[MAX];
  long int ntel;
  long int age;
};

[toc] | [next] | [standalone]

#7014

On 06/27/11 03:50 PM, Vinicio Flores wrote:
> Hello C hackers, this program that I am writing print "Segmentation
> fault" when I execute it, I think that the problem is the gets
> function,  but I am not sure

Don't use gets(), use fgets().

<snip>

> static int open_and_fill_database(char *arg)
> {
>    FILE *database;
>
>    struct datbase_registers *base = malloc ( sizeof(struct registers
> *));

You should check to see if this succeeds!

<snip>

> #define MAX    ((900*800)*256)

Are you serious? 184 million!


> struct datbase_registers
> {
>    char regname[MAX];
>    char name[MAX];
>    long int ntel;
>    long int age;
> };

So one of these is > 400MB!

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#7039

Am 27.06.2011 06:07, schrieb Ian Collins:
> On 06/27/11 03:50 PM, Vinicio Flores wrote:
>> Hello C hackers, this program that I am writing print "Segmentation
>> fault" when I execute it, I think that the problem is the gets
>> function, but I am not sure
>
> Don't use gets(), use fgets().
>
> <snip>
>
>> static int open_and_fill_database(char *arg)
>> {
>> FILE *database;
>>
>> struct datbase_registers *base = malloc ( sizeof(struct registers
>> *));
>
> You should check to see if this succeeds!
>
of course, you should get enough memory for a struct registers variable 
instead of a struct registers POINTER variable!
i.e.
struct database_registers *base =
   malloc(sizeof(struct database_registers));
Good luck!
> <snip>
>
>> #define MAX ((900*800)*256)
>
> Are you serious? 184 million!
>
>
>> struct datbase_registers
>> {
>> char regname[MAX];
>> char name[MAX];
>> long int ntel;
>> long int age;
>> };
>
> So one of these is > 400MB!
>

[toc] | [prev] | [next] | [standalone]

#7041

On 06/27/11 09:37 PM, Dirk Zabel wrote:
> Am 27.06.2011 06:07, schrieb Ian Collins:
>> On 06/27/11 03:50 PM, Vinicio Flores wrote:
>>> Hello C hackers, this program that I am writing print "Segmentation
>>> fault" when I execute it, I think that the problem is the gets
>>> function, but I am not sure
>>
>> Don't use gets(), use fgets().
>>
>> <snip>
>>
>>> static int open_and_fill_database(char *arg)
>>> {
>>> FILE *database;
>>>
>>> struct datbase_registers *base = malloc ( sizeof(struct registers
>>> *));
>>
>> You should check to see if this succeeds!
>>
> of course, you should get enough memory for a struct registers variable
> instead of a struct registers POINTER variable!
> i.e.
> struct database_registers *base =
>     malloc(sizeof(struct database_registers));

Good spot, I was fooled by the line-wrap.

The problem wouldn't have arisen if he'd written

struct datbase_registers *base = malloc( sizeof *base );

Which is why people here recommend that form.

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#7046

"Vinicio Flores" <vfloreshdz@gmail.com> wrote in message 
news:e4f31412-ae7a-455f-9296-3cca0bbccedb@g16g2000yqg.googlegroups.com...
> Hello C hackers, this program that I am writing print "Segmentation
> fault" when I execute it, I think that the problem is the gets
> function,  but I am not sure
>
> /** A simple database system **/
>
> #include "datbase.h"
>
> static int search_and_open_data_base(char *Arg)
> {
>  FILE *f;
>
>  f = fopen(Arg, "r");
>
>  if(!f)
>    return 1;
>  else
>    return 0;
> }

Since you can't access the file after returning, you might as well close it 
first:

 fclose(f);
 return 1;

since re-opening later could cause problems.

-- 
Bartc 

[toc] | [prev] | [next] | [standalone]

#7051

On Sun, 26 Jun 2011 20:50:21 -0700 (PDT), Vinicio Flores
<vfloreshdz@gmail.com> wrote:

>Hello C hackers, this program that I am writing print "Segmentation
>fault" when I execute it, I think that the problem is the gets
>function,  but I am not sure

Your functions do not play nicely together.  You have too many calls
to fopen() and the modes do not match the intended use.

>
>/** A simple database system **/
>
>#include "datbase.h"
>
>static int search_and_open_data_base(char *Arg)

This function does not do any searching.

>{
>  FILE *f;

Don't you need to #include stdio.h for FILE and its associated
functions such as fopen?

>
>  f = fopen(Arg, "r");
>
>  if(!f)
>    return 1;
>  else
>    return 0;

Now you have an open file and no FILE* that points to it.  You can
never close it.  (This is a possible memory leak depending on how your
system allocates files.)  On some system, a second attempt to open the
file for output will fail because it is already open for input.

>}
>
>/* ************************************************ */
>
>static FILE *create_database(char *arg)
>{
>  FILE *newbase;
>
>  newbase = fopen(arg, "w+");
>
>  return newbase;
>}
>
>/* ************************************************ */
>
>static int print_options()
>{
>  clrscr();
>  setattr(REVERSE);
>  setcolor(FOREBLUE);
>
>  gotoxy(2, 2);
>  cprintf("DatBASE");

That is a strange use of camel case.

You use a lot of non-standard functions (possibly POSIX) but you don't
seem to have the correct headers for them.  In any event, this limits
the number of people who can attempt to help to just those who have
the same or a similar system.  If that is essential to understanding
the problem, you might be better off posting in a group that deals
with that system.

In this case, why use a non-standard function which does nothing
different than a standard function?

>
>  gotoxy(2, 4);
>  setattr(NORMAL);
>  setcolor(FOREGREEN);
>  cprintf("I)Insert new register  R)Print a register S) Exit");

Most menus seem to use P for Print and X for eXit.

>
>  gotoxy(2, 6);
>  setcolor(FORERED);
>  cprintf("Option: ");
>
>  setattr(NORMAL);
>  normvideo();
>
>}
>
>/* ************************************************ */
>
>static int open_and_fill_database(char *arg)
>{
>  FILE *database;
>
>  struct datbase_registers *base = malloc ( sizeof(struct registers
>*));

Others have pointed out the error here.

>
>  database = fopen(arg, "ra");

ra is not a standard mode.  Read and append makes no sense.

>
>  gotoy(10);
>  puts("Insert register name: ");
>  gets(base->regname);

Perhaps the only time no one will ever mention gets() overrunning the
target array.  When you fix the size of your arrays, fgets() will be a
better choice.

>
>  gotoy(11);
>  puts("Insert name: ");
>  gets(base->name);
>
>  gotoy(12);
>  puts("Insert telephone number: ");
>  scanf("%ld", base->ntel);

Storing a telephone number as an integer can produce misleading
results if phone numbers start with 0 (as in Europe and Japan).

>
>  gotoy(13);
>  puts("Insert age: ");
>  scanf("%ld", &base->age);

You really expect age to exceed 65K?

>
>  fprintf(database, "Register name: ");
>  fputs(base->regname, database);

If you are going to bear the expense of calling fprintf, why not
include everything in one function call
   fprintf(database, "Register name: %s\n", base->regname);
and eliminate the call to fputs?

>
>  fprintf(database, "\n\nPerson's name: ");
>  fputs(base->name, database);
>
>  fprintf(database, "\nTelephone number: %lf\n", base->ntel);

Here you invoke undefined behavior.  ntel is an integer, not a
floating point.  Did you mean %ld?

>
>  fclose(database);

You never stored age in database.

>}
>
>/* ************************************************ */
>
>static int get_and_process_input()
>{
>  int ch;
>
>  ch = getch();

The last input could have been age.  scanf() would have left the '\n'
from the ENTER key sitting in the input buffer.  That is the character
that this getch() would pick up.  Not what you want.

>
>  switch(ch)
>    {
>    case 'i':
>    case 'I':
>      open_and_fill_database("data.base");
>      break;
>    case 'r':
>    case 'R':
>      break;
>    case 's':
>    case 'S':
>      setattr(NORMAL);
>      normvideo();
>      reset();
>      ret;

What kind of statement is this?  Is it supposed to be a function call:
    ret();

>      break;
>    default:
>      gotoy(9);
>      cprintf("?Unknown operation\n");
>      sleep(1);
>      print_options();
>      get_and_process_input();

Did you really mean to be recursive?  Wouldn't a while loop be much
easier to debug?

>      break;
>    }

More undefined behavior.  This function is defined to return an int.
You don't.  Didn't your compiler generate a diagnostic for this?  If
not, maybe you should up the warning level.

>}
>
>/* ********************|*************************** */
>/* --------------------|--------------------------- */
>/* ************| MAIN FUNCTION |******************* */
>
>int main()
>{
>  FILE *database;

You really need to use this variable somewhere.

>
>  int c;
>  int search = search_and_open_data_base("data.base");
>
>
>  if(search == 1)
>    create_database("data.base");

You created the file but threw away the FILE* the function returned.
You now have an empty file opened for output that you have no way to
process or close.

>
>  else
>    {

At this point, the file is open for input.

>      print_options();
>      get_and_process_input();

This may call open_and_fill_database() which will always attempt to
open data.base even if you change the file name later.  This is one
occasion where having pointer to (or array containing) the file name
at file scope would be not be unreasonable, especially if it were
const.

>    }
>  return 0;
>}
>
>
>Here is the datbase.h  C header:
>/** A simple database system **/
>
>#include <ttk.h>
>#include <unistd.h>
>#include <sys/types.h>
>#include <sys/stat.h>
>#include <fcntl.h>
>
>#define MAX    ((900*800)*256)

What benefit do you perceive from the extraneous parentheses?

>
>struct datbase_registers
>{
>  char regname[MAX];
>  char name[MAX];
>  long int ntel;
>  long int age;
>};

-- 
Remove del for email

[toc] | [prev] | [next] | [standalone]

#7105

"Vinicio Flores" <vfloreshdz@gmail.com> schrieb im Newsbeitrag 
news:e4f31412-ae7a-455f-9296-3cca0bbccedb@g16g2000yqg.googlegroups.com...
...
> static FILE *create_database(char *arg)
> {
>  FILE *newbase;
>
>  newbase = fopen(arg, "w+");
>
>  return newbase;
> }
...
newbase resides on the stack in the scope of create_database. After you 
leave create_database, the memory space allocated for newbase is lost! You 
return the pointer newbase to the caller of create_database, but after you 
have left create_database, it points to freed memory. If the caller uses 
this pointer, this leads to a segmentation fault!

static FILE *newbase;
will solve that problem.

kind regards
Heiner 

[toc] | [prev] | [next] | [standalone]

#7106

On June 27, 2011 17:54, in comp.lang.c, invalid@invalid.invalid wrote:

> 
> "Vinicio Flores" <vfloreshdz@gmail.com> schrieb im Newsbeitrag
> news:e4f31412-ae7a-455f-9296-3cca0bbccedb@g16g2000yqg.googlegroups.com...
> ...
>> static FILE *create_database(char *arg)
>> {
>>  FILE *newbase;
>>
>>  newbase = fopen(arg, "w+");
>>
>>  return newbase;
>> }
> ...
> newbase resides on the stack in the scope of create_database.

True, but the /pointer/ stored in newbase (on successful operation of
fopen() ) points to a value outside the scope of create_database().

> After you 
> leave create_database, the memory space allocated for newbase is lost! 

True that newbase is no longer referencable after create_database() ends,
but the pointer stored in newbase doesn't point to anything within
create_database()'s scope. Instead, the /pointer/ points to something
specifically outside the scope of create_database(), and that something
does not "go away" when create_database() terminates. Thus, the /pointer/
is still valid, even after the termination of create_database().

> You 
> return the pointer newbase to the caller of create_database, but after you
> have left create_database, it points to freed memory.

No, it doesn't.

> If the caller uses 
> this pointer, this leads to a segmentation fault!

No it won't.


-- 
Lew Pitcher
Master Codewright & JOAT-in-training   | Registered Linux User #112576
Me: http://pitcher.digitalfreehold.ca/ | Just Linux: http://justlinux.ca/
----------      Slackware - Because I know what I'm doing.         ------

[toc] | [prev] | [next] | [standalone]

#7110

On Mon, 27 Jun 2011 23:54:08 +0200, "Heinrich Wolf"
<invalid@invalid.invalid> wrote:

>
>"Vinicio Flores" <vfloreshdz@gmail.com> schrieb im Newsbeitrag 
>news:e4f31412-ae7a-455f-9296-3cca0bbccedb@g16g2000yqg.googlegroups.com...
>...
>> static FILE *create_database(char *arg)
>> {
>>  FILE *newbase;
>>
>>  newbase = fopen(arg, "w+");
>>
>>  return newbase;
>> }
>...
>newbase resides on the stack in the scope of create_database. After you 
>leave create_database, the memory space allocated for newbase is lost! You 
>return the pointer newbase to the caller of create_database, but after you 
>have left create_database, it points to freed memory. If the caller uses 
>this pointer, this leads to a segmentation fault!

Not at all.  While it is true that newbase ceases to exist when the
function returns, the memory  it points to does not.  In fact, the
memory it points to was determined by fopen and if that memory were to
be freed automatically at any point, it would be at the end of fopen.

>
>static FILE *newbase;
>will solve that problem.

Completely unnecessary since there is no need for newbase to continue
to exist.  Its value is what is returned and that value remains valid
until a call to fclose.

-- 
Remove del for email

[toc] | [prev] | [next] | [standalone]

#7187

"Heinrich Wolf" <invalid@invalid.invalid> writes:
> "Vinicio Flores" <vfloreshdz@gmail.com> schrieb im Newsbeitrag 
> news:e4f31412-ae7a-455f-9296-3cca0bbccedb@g16g2000yqg.googlegroups.com...
> ...
>> static FILE *create_database(char *arg)
>> {
>>  FILE *newbase;
>>
>>  newbase = fopen(arg, "w+");
>>
>>  return newbase;
>> }
> ...
> newbase resides on the stack in the scope of create_database. After you 
> leave create_database, the memory space allocated for newbase is lost! You 
> return the pointer newbase to the caller of create_database, but after you 
> have left create_database, it points to freed memory. If the caller uses 
> this pointer, this leads to a segmentation fault!
>
> static FILE *newbase;
> will solve that problem.

Returning the value of a local object returns *a copy* of that value.
It's not necessary for the object to continue to exist after that.  It's
as true for pointer objects as for, say, int objects.

int func(void)
{
    int local = 42;
    return local;
}

Here, "local" no longer exists, but func1() has no problem returning the
value 42.

It's returning the *address* of a local variable that can cause problems.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Nokia
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#7203

"Heinrich Wolf" <invalid@invalid.invalid> schrieb im Newsbeitrag 
news:iuau65$5fc$1@news-1.m-online.net...
>
> ...
> newbase resides on the stack in the scope of create_database. After you 
> leave create_database, the memory space allocated for newbase is lost! You 
> return the pointer newbase to the caller of create_database, but after you 
> have left create_database, it points to freed memory. If the caller uses 
> this pointer, this leads to a segmentation fault!
>
> static FILE *newbase;
> will solve that problem.
>
> kind regards
> Heiner

I'm sorry!

I mixed it up with someting like this:

char *SomeFunction(void)
  {
    char Buffer[10];
    strcpy(Buffer, "Hello");
    return Buffer;
  }

Here you need
static char Buffer[10]; 

[toc] | [prev] | [next] | [standalone]

#7219

"Heinrich Wolf" <invalid@invalid.invalid> writes:
> "Heinrich Wolf" <invalid@invalid.invalid> schrieb im Newsbeitrag 
> news:iuau65$5fc$1@news-1.m-online.net...
>>
>> ...
>> newbase resides on the stack in the scope of create_database. After you 
>> leave create_database, the memory space allocated for newbase is lost! You 
>> return the pointer newbase to the caller of create_database, but after you 
>> have left create_database, it points to freed memory. If the caller uses 
>> this pointer, this leads to a segmentation fault!
>>
>> static FILE *newbase;
>> will solve that problem.
>
> I'm sorry!
>
> I mixed it up with someting like this:
>
> char *SomeFunction(void)
>   {
>     char Buffer[10];
>     strcpy(Buffer, "Hello");
>     return Buffer;
>   }
>
> Here you need
> static char Buffer[10]; 

Or one of several other possibilities, such as having the caller pass
in a pointer to an array that will hold the result, or allocating
the result with malloc() and making the caller responsible for
free()ing it.

C's special-case semantics here lay a trap for the unwary.  The
statement "return Buffer;" *looks* like it's innocuously returning
the value of a local object, but the implicit array-to-pointer
conversion means it's really returning the address of a local objec.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Nokia
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web