destroying objects

Discussion in 'C Programming' started by pereges, Jun 11, 2008.

  1. pereges

    pereges Guest

    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.eid, &list->e.age, &list-
    >e.sal);

    }

    for(i = 0; i < list->n; i++)
    {
    printf("Employe id: %d age: %d sal: %f\n", list->e.eid, list-
    >e.age, list->e.sal);

    }

    list->free_employee_list(list);

    return 0;
    }
    pereges, Jun 11, 2008
    #1
    1. Advertising

  2. On Wed, 11 Jun 2008 11:11:24 -0700 (PDT), pereges <>
    wrote:

    >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.

    >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.

    >
    >#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);

    >}
    >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.

    > return 1;


    To be portable use EXIT_FAILURE.

    > }
    >
    > 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.

    > return 1;
    > }
    >
    > for(i = 0; i < list->n; i++)
    > {
    > printf("Enter id, age, salary\n");
    > scanf("%d %d %f", &list->e.eid, &list->e.age, &list-
    >>e.sal);

    > }
    >
    > for(i = 0; i < list->n; i++)
    > {
    > printf("Employe id: %d age: %d sal: %f\n", list->e.eid, list-
    >>e.age, list->e.sal);

    > }
    >
    > list->free_employee_list(list);


    By not freeing list->e before freeing list, you have created a memory
    leak.

    >
    > return 0;
    >}



    Remove del for email
    Barry Schwarz, Jun 12, 2008
    #2
    1. Advertising

  3. On 11 Jun, 19:11, pereges <> wrote:

    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).

    <snip>

    >   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...)
    Nick Keighley, Jun 12, 2008
    #3
  4. CBFalconer <> writes:
    > Chris Thomasson wrote:
    >> "Nick Keighley" <> wrote:
    >>> pereges <> 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) <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"
    Keith Thompson, Jun 13, 2008
    #4
  5. CBFalconer <> writes:
    > Chris Thomasson wrote:
    >> "CBFalconer" <> wrote in message
    >>> Chris Thomasson wrote:
    >>>

    > ... snip ...
    >>>
    >>>> 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?

    >
    > When macros are expanded the system has not parsed the function
    > header. It is just so much more text to be treated according to
    > the macro rules.


    But that's not an issue in this case, since <stdio.h> cannot legally
    define _this as a macro. See my other response.

    --
    Keith Thompson (The_Other_Keith) <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"
    Keith Thompson, Jun 13, 2008
    #5
  6. On Thu, 12 Jun 2008 18:02:11 -0700, Keith Thompson wrote:
    > CBFalconer <> writes:
    >> Chris Thomasson wrote:
    >>> Something like the following is perfectly fine:
    >>>
    >>> #include <stdio.h>
    >>>
    >>> void foo(int _this) {
    >>> printf("%d\n", _this);
    >>> }
    >>> [...]

    >> 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". [snip explanation]


    Yes, it can: _this is reserved for use by the implementation at file
    scope. This means an implementation is allowed to provide a declaration of
    a function named _this in <stdio.h>. And 7.1.4 states that "Any function
    declared in a header may be additionally implemented as a function-like
    macro defined in the header," which does not only apply to the functions
    described by the standard. Existing implementations make use of this
    permission with macro definitions of _toupper and _tolower in <ctype.h>.

    That said, <stdio.h> cannot legally define an object-like macro named
    "_this", so the code by Chris Thomasson is still perfectly fine.
    Harald van Dijk, Jun 13, 2008
    #6
  7. Harald van Dijk <> writes:
    > On Thu, 12 Jun 2008 18:02:11 -0700, Keith Thompson wrote:
    >> CBFalconer <> writes:
    >>> Chris Thomasson wrote:
    >>>> Something like the following is perfectly fine:
    >>>>
    >>>> #include <stdio.h>
    >>>>
    >>>> void foo(int _this) {
    >>>> printf("%d\n", _this);
    >>>> }
    >>>> [...]
    >>> 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". [snip explanation]

    >
    > Yes, it can: _this is reserved for use by the implementation at file
    > scope. This means an implementation is allowed to provide a declaration of
    > a function named _this in <stdio.h>. And 7.1.4 states that "Any function
    > declared in a header may be additionally implemented as a function-like
    > macro defined in the header," which does not only apply to the functions
    > described by the standard. Existing implementations make use of this
    > permission with macro definitions of _toupper and _tolower in <ctype.h>.


    Interesting. (I overlooked function-like macros.) I think you're
    right, but I wonder if that was the intent. Perhaps the committee
    just didn't think about macros for non-standard functions in standard
    headers.

    It would have been just as easy for implementations to use __toupper
    and __tolower.

    > That said, <stdio.h> cannot legally define an object-like macro named
    > "_this", so the code by Chris Thomasson is still perfectly fine.


    Right.

    --
    Keith Thompson (The_Other_Keith) <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"
    Keith Thompson, Jun 13, 2008
    #7
  8. On Fri, 13 Jun 2008 09:49:28 -0700, Keith Thompson wrote:
    > Harald van Dijk <> writes:
    >> Existing implementations
    >> make use of this permission with macro definitions of _toupper and
    >> _tolower in <ctype.h>.

    >
    > It would have been just as easy for implementations to use __toupper and
    > __tolower.


    Normally, this would be true, but _toupper and _tolower predate the
    original C standard by several years. According to the Single UNIX
    Specification, they are derived from Issue 1 of the SVID, which is from
    1985. It would have been easier for the C standard to define these
    functions than for implementations to rename them.
    Harald van Dijk, Jun 13, 2008
    #8
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. NETUser2004
    Replies:
    3
    Views:
    468
    John Saunders
    Feb 10, 2004
  2. =?Utf-8?B?dnZlbms=?=

    Destroying objects in Page_Unload event

    =?Utf-8?B?dnZlbms=?=, Nov 9, 2005, in forum: ASP .Net
    Replies:
    3
    Views:
    547
    Kevin Spencer
    Nov 9, 2005
  3. Varangian

    Destroying Sessions

    Varangian, Mar 29, 2006, in forum: ASP .Net
    Replies:
    6
    Views:
    481
    Juan T. Llibre
    Mar 29, 2006
  4. molar
    Replies:
    0
    Views:
    611
    molar
    Jul 25, 2004
  5. Markus Pitha
    Replies:
    8
    Views:
    280
    benben
    Oct 29, 2007
Loading...

Share This Page