|  | destroying objects |  | |
| | | pereges |  |
| Posted: Wed Jun 11, 2008 6:11 pm Post subject: destroying objects |  |
| |  | |
This program simply reads a list of employes from the user, prints it and then destroys the allocated memory. I have a function free_memory(void *ptr) which will free the memory pointed to by pointer ptr. I included a pointer to this function in the emplist data structure free_employee_list. Is this a good way to program ?? what if there are a number of such different data structures. Would it suffice to have a single free_memory function throughout the project and include a pointer to this function in every data structure ??
#include <stdio.h> #include <stdlib.h>
typedef struct _employee { int eid; float sal; int age; }employee;
typedef struct _emplist { int n; employee *e; void (*free_employee_list)(); }emplist;
void free_memory(void *ptr) { free(ptr); printf("\nMemory freed\n"); } int main(void) { emplist *list; int i;
list = malloc(sizeof(*list)); if(list == NULL) { fprintf(stderr, "memory allocation failed\n"); return 1; }
list->free_employee_list = free_memory; printf("Enter number of employees\n"); scanf("%d", &list->n);
list->e = malloc(sizeof(employee) * list->n);
if(list->e == NULL) { fprintf(stderr, "memory allocation failed\n"); return 1; }
for(i = 0; i < list->n; i++) { printf("Enter id, age, salary\n"); scanf("%d %d %f", &list->e[i].eid, &list->e[i].age, &list-
for(i = 0; i < list->n; i++) { printf("Employe id: %d age: %d sal: %f\n", list->e[i].eid, list-
| Quote: | e[i].age, list->e[i].sal); } |
list->free_employee_list(list);
return 0; } |
| |
| | | Malcolm McLean |  |
| Posted: Wed Jun 11, 2008 8:28 pm Post subject: Re: destroying objects |  |
"pereges" <Broli00@gmail.com> wrote in message news:
| Quote: | This program simply reads a list of employes from the user, prints it and then destroys the allocated memory. I have a function free_memory(void *ptr) which will free the memory pointed to by pointer ptr. I included a pointer to this function in the emplist data structure free_employee_list. Is this a good way to program ??
It's (arguably) a good way to program, but not in C. C's syntax isn't rich |
enough to support a heavily indirected set of function pointers embedded in their own structures. It's an easy route to unreadability.
However the same idea in C++ is fine. The language will call destructors automatically for you.
-- Free games and programming goodies. LINK |
| |
| | | pete |  |
| Posted: Thu Jun 12, 2008 3:47 am Post subject: Re: destroying objects |  |
| |  | |
pereges wrote:
| Quote: | This program simply reads a list of employes from the user, prints it and then destroys the allocated memory. I have a function free_memory(void *ptr) which will free the memory pointed to by pointer ptr. I included a pointer to this function in the emplist data structure free_employee_list. Is this a good way to program ?? what if there are a number of such different data structures. Would it suffice to have a single free_memory function throughout the project and include a pointer to this function in every data structure ??
#include <stdio.h #include <stdlib.h
typedef struct _employee { int eid; float sal; int age; }employee;
typedef struct _emplist { int n; employee *e; void (*free_employee_list)(); }emplist;
void free_memory(void *ptr) { free(ptr); printf("\nMemory freed\n"); } int main(void) { emplist *list; int i;
list = malloc(sizeof(*list)); if(list == NULL) { fprintf(stderr, "memory allocation failed\n"); return 1; }
list->free_employee_list = free_memory; printf("Enter number of employees\n"); scanf("%d", &list->n);
list->e = malloc(sizeof(employee) * list->n);
if(list->e == NULL) { fprintf(stderr, "memory allocation failed\n"); return 1; }
for(i = 0; i < list->n; i++) { printf("Enter id, age, salary\n"); scanf("%d %d %f", &list->e[i].eid, &list->e[i].age, &list- e[i].sal); }
for(i = 0; i < list->n; i++) { printf("Employe id: %d age: %d sal: %f\n", list->e[i].eid, list- e[i].age, list->e[i].sal); }
list->free_employee_list(list);
return 0; }
|
It looks overly complicated to me. I don't understand why it isn't just as simple as allocating an array of structures and then freeing it.
/* BEGIN employees_2.c */
#include <stdio.h> #include <stdlib.h>
int main(void) { struct { int eid; double sal; int age; } *e_ptr; unsigned i, n;
puts("/* BEGIN employees_2.c output */\n"); puts("Enter number of employees"); if (scanf("%u", &n) != 1) { puts("Forget about it."); exit(EXIT_SUCCESS); } e_ptr = malloc(n * sizeof *e_ptr); if (e_ptr == NULL) { puts("e_ptr == NULL"); exit(EXIT_FAILURE); }
for (i = 0; i != n; ++i) { puts("Enter id, age, salary"); if (scanf("%d %d %lf", &e_ptr[i].eid, &e_ptr[i].age, &e_ptr[i].sal) != 3) { puts("Forget about it."); break; } } putchar('\n'); for (n = 0; i != n; ++n) { printf("Employe id: %d age: %d sal: %f\n", e_ptr[n].eid, e_ptr[n].age, e_ptr[n].sal); } free(e_ptr); puts("\nArray is freed.\n"); puts("/* END employees_2.c output */"); return 0; }
/* END employees_2.c */
-- pete |
| |
| | | Barry Schwarz |  |
| Posted: Thu Jun 12, 2008 4:07 am Post subject: Re: destroying objects |  |
| |  | |
On Wed, 11 Jun 2008 11:11:24 -0700 (PDT), pereges <Broli00@gmail.com> wrote:
| Quote: | This program simply reads a list of employes from the user, prints it and then destroys the allocated memory. I have a function free_memory(void *ptr) which will free the memory pointed to by pointer ptr. I included a pointer to this function in the emplist data structure free_employee_list. Is this a good way to program ??
|
Is it a good idea to have a function whose only purpose is to call a standard function? I think not.
Is it a good idea to have a function pointer point to this function? Not unless you have more than one such function and the point where you decide which one to use is removed from the point where you call the function.
Is it a good idea to have this pointer as a member of the struct _emplist? Probably, since everything else you need to "control" struct _employee is included in struct _emplist.
| Quote: | what if there are a number of such different data structures. Would it suffice to have a single free_memory function throughout the project and include a pointer to this function in every data structure ??
|
If you only have one function, why bother wasting the space for a pointer to it in every data structure.
| Quote: | #include <stdio.h #include <stdlib.h
typedef struct _employee { int eid; float sal; int age; }employee;
typedef struct _emplist { int n; employee *e; void (*free_employee_list)(); }emplist;
void free_memory(void *ptr) { free(ptr); printf("\nMemory freed\n");
|
It would be nice if you said what memory was being freed. Something like printf("\nMemory allocated at %p freed\n", ptr); free(ptr);
| Quote: | } int main(void) { emplist *list; int i;
list = malloc(sizeof(*list)); if(list == NULL) { fprintf(stderr, "memory allocation failed\n");
|
It would be nice to indicate which allocation failed.
To be portable use EXIT_FAILURE.
| Quote: | }
list->free_employee_list = free_memory; printf("Enter number of employees\n"); scanf("%d", &list->n);
list->e = malloc(sizeof(employee) * list->n);
if(list->e == NULL) { fprintf(stderr, "memory allocation failed\n");
|
You ought to free list at this point.
| Quote: | return 1; }
for(i = 0; i < list->n; i++) { printf("Enter id, age, salary\n"); scanf("%d %d %f", &list->e[i].eid, &list->e[i].age, &list- e[i].sal); }
for(i = 0; i < list->n; i++) { printf("Employe id: %d age: %d sal: %f\n", list->e[i].eid, list- e[i].age, list->e[i].sal); }
list->free_employee_list(list);
|
By not freeing list->e before freeing list, you have created a memory leak.
Remove del for email |
| |
| | | Nick Keighley |  |
| Posted: Thu Jun 12, 2008 8:16 am Post subject: Re: destroying objects |  |
| |  | |
On 11 Jun, 19:11, pereges <Brol...@gmail.com> wrote:
I agree with the other posters
<snip>
| Quote: | typedef struct _employee
|
identifiers beginning with _ are in the reserved namespace. That is only compiler writters and standard library implementors should use it. (The rules are slightly more complicated but "don't begin identifiers with underscore" is easy to remember).
<snip>
| Quote: | printf("Enter number of employees\n"); scanf("%d", &list->n);
list->e = malloc(sizeof(employee) * list->n);
|
failure to test return value of scanf(). scanf() is tricky to use, its error recovery is poor (try entering a string of letters). Use fgets() and fscanf().
<snip>
-- Nick Keighley
The fscanf equivalent of fgets is so simple that it can be used inline whenever needed:- char s[NN + 1] = "", c; int rc = fscanf(fp, "%NN[^\n]%1[\n]", s, &c); if (rc == 1) fscanf("%*[^\n]%*c); if (rc == 0) getc(fp);
(actually it can't be *that* simple as I've just spotted two syntax errors...) |
| |
| | | Chris Thomasson |  |
| Posted: Thu Jun 12, 2008 7:44 pm Post subject: Re: destroying objects |  |
"Nick Keighley" <nick_keighley_nospam@hotmail.com> wrote in message news:3cee76d5-98b3-4978-8992-f21c281c2428@x41g2000hsb.googlegroups.com... On 11 Jun, 19:11, pereges <Brol...@gmail.com> wrote:
| Quote: | I agree with the other posters
snip
typedef struct _employee
identifiers beginning with _ are in the reserved namespace. That is only compiler writters and standard library implementors should use it. (The rules are slightly more complicated but "don't begin identifiers with underscore" is easy to remember).
|
Something like the following is perfectly fine: _________________________________________________ #include <stdio.h>
void foo(int _this) { printf("%d\n", _this); }
int main(void) { foo(1); getchar(); return 0; }
_________________________________________________
LINK
[...] |
| |
| | | CBFalconer |  |
| Posted: Thu Jun 12, 2008 8:30 pm Post subject: Re: destroying objects |  |
Chris Thomasson wrote:
| Quote: | "Nick Keighley" <nick_keighley_nospam@hotmail.com> wrote: pereges <Brol...@gmail.com> wrote:
.... snip ...
typedef struct _employee
identifiers beginning with _ are in the reserved namespace. That is only compiler writters and standard library implementors should use it. (The rules are slightly more complicated but "don't begin identifiers with underscore" is easy to remember).
Something like the following is perfectly fine:
#include <stdio.h
void foo(int _this) { printf("%d\n", _this); }
int main(void) { foo(1); getchar(); return 0; }
|
Not if stdio.h has (legitimately) defined a macro for _this. Using plain this is safer, since the scope of this only extends to the function closing '}'.
-- [mail]: Chuck F (cbfalconer at maineline dot net) [page]: <http://cbfalconer.home.att.net> Try the download section.
** Posted from LINK ** |
| |
| | | Chris Thomasson |  |
| Posted: Thu Jun 12, 2008 9:23 pm Post subject: Re: destroying objects |  |
| |  | |
"CBFalconer" <cbfalconer@yahoo.com> wrote in message news:4851A39C.89D36C85@yahoo.com...
| Quote: | Chris Thomasson wrote: "Nick Keighley" <nick_keighley_nospam@hotmail.com> wrote: pereges <Brol...@gmail.com> wrote:
... snip ...
typedef struct _employee
identifiers beginning with _ are in the reserved namespace. That is only compiler writters and standard library implementors should use it. (The rules are slightly more complicated but "don't begin identifiers with underscore" is easy to remember).
Something like the following is perfectly fine:
#include <stdio.h
void foo(int _this) { printf("%d\n", _this); }
int main(void) { foo(1); getchar(); return 0; }
Not if stdio.h has (legitimately) defined a macro for _this. Using plain this is safer, since the scope of this only extends to the function closing '}'.
|
Ouch. Well, I thought that identifiers which start with a double underscore, single underscore followed by a uppercase character, or a single underscore at file scope are not allowed. Since function parameters are not at file scope, well, using `_this' should be fine. Where am I going wrong?
Thanks. |
| |
| | | Chris Thomasson |  |
| Posted: Thu Jun 12, 2008 9:37 pm Post subject: Re: destroying objects |  |
"CBFalconer" <cbfalconer@yahoo.com> wrote in message news:4851A39C.89D36C85@yahoo.com...
| Quote: | Chris Thomasson wrote: "Nick Keighley" <nick_keighley_nospam@hotmail.com> wrote: pereges <Brol...@gmail.com> wrote:
... snip ...
typedef struct _employee
identifiers beginning with _ are in the reserved namespace. That is only compiler writters and standard library implementors should use it. (The rules are slightly more complicated but "don't begin identifiers with underscore" is easy to remember).
Something like the following is perfectly fine:
#include <stdio.h
void foo(int _this) { printf("%d\n", _this); }
int main(void) { foo(1); getchar(); return 0; }
Not if stdio.h has (legitimately) defined a macro for _this. Using plain this is safer, since the scope of this only extends to the function closing '}'.
|
I agree that using `this' would be a heck of a lot safer. Its been a nasty habit of mine to use `_this'.
;^(... |
| |
| | | Keith Thompson |  |
| Posted: Thu Jun 12, 2008 11:02 pm Post subject: Re: destroying objects |  |
| |  | |
CBFalconer <cbfalconer@yahoo.com> writes:
| Quote: | Chris Thomasson wrote: "Nick Keighley" <nick_keighley_nospam@hotmail.com> wrote: pereges <Brol...@gmail.com> wrote:
... snip ...
typedef struct _employee
identifiers beginning with _ are in the reserved namespace. That is only compiler writters and standard library implementors should use it. (The rules are slightly more complicated but "don't begin identifiers with underscore" is easy to remember).
Something like the following is perfectly fine:
#include <stdio.h
void foo(int _this) { printf("%d\n", _this); }
int main(void) { foo(1); getchar(); return 0; }
Not if stdio.h has (legitimately) defined a macro for _this. Using plain this is safer, since the scope of this only extends to the function closing '}'.
|
stdio.h cannot legally define a macro named "_this". As Nick wrote above, the rules are "slightly more complicated". Specifically (C99 7.1.3):
Identifiers beginning with an underscore and an uppercase letter, or with two underscores, are always reserved for any use. For example, stdio.h could define a macro "__this" or "_This".
Identifiers beginning with an underscore are reserved for use as identifiers with file scope. For example, stdio.h could declare a function "_this".
But since "_this" in Chris's code is not at file scope, it doesn't violate one of the standard's reservations.
The above program is perfectly legal, and in fact it's arguably strictly conforming (ignoring the possibility of printf failing).
However, I wouldn't call it "perfectly fine" on stylistic grounds. First, I prefer to avoid declaring identifiers starting with underscores altogether, since it's easier than keeping track of the distinction between the two cases I cited above (and expecting anyone reading the code to do so as well). It's not as if avoiding leading underscores places a burdensome limitation on the set of identifiers I can use.
Second, the ``getchar()'' call is superfluous; all it does is silently wait for user input before terminating the program, which is just annoying. (Yes, there are systems where this is useful, but there are other ways to achieve the same effect.)
-- 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" |
| |
| Page 1 of 2 .:. Goto page 1, 2 Next | |
|
|