3 ms·

About the patch at https://github.com/vmg/redcarpet/pull/516/files https://github.com/vmg/redcarpet/pull/516/files, couldn't the update simply have been the fol

by bru 10y ago

About the patch at https://github.com/vmg/redcarpet/pull/516/files https://github.com/vmg/redcarpet/pull/516/files, couldn't the update simply have been the following? - return Data_Wrap_Struct(klass, rb_redcarpet_rbase_mark, NULL, rndr); + return Data_Wrap_Struct(klass, rb_redcarpet_rbase_mark, xfree, rndr);

busterarm 10y ago
C is not really my area of expertise, but doesn't xfree have alternatives? like free, freeif, etc? I'd much rather have to change that later in one place than several (if there end up being more calls to xfree in the code) if at some point the implementation needs to change again.
- simcop2387 10y ago
  So a from a quick googling it looks like xfree()[1] is a ruby specific thing, and should be used internally for everything allocated by ruby. It also looks like it's got the same signature as the function created for this so that should work there too. [2] [1] http://clalance.blogspot.com/2011/01/writing-ruby-extensions-in-c-part-12.html http://clalance.blogspot.com/2011/01/writing-ruby-extensions... [2] http://inferior-products.com/docs/userdocs/ruby19/html/d8/d16/gc_8c.html#a0bffec5b2cc004adcebb6802e7620387 http://inferior-products.com/docs/userdocs/ruby19/html/d8/d1...
  - busterarm 10y ago
    ...Are you sure? I was pretty sure that it gets it from the same place as xmalloc...wouldn't that be in libc?
    - mrud 10y ago
      There is no xmalloc in the libc.
      - busterarm 10y ago
        Okay, interesting. I guess I've seen it before, so I didn't think it was Ruby-specific, but I also haven't had to do that kind of programming in a very super long time. Publib then? I'm finding this when I search: http://man.cx/xfree(3) http://man.cx/xfree(3) http://man.cx/publib(3) http://man.cx/publib(3) Also, are you sure? I see xmalloc in glibc all over the internet...
        kristianp 10y ago
        edit: actually xmalloc and xfree are defined as #define xmalloc ruby_xmalloc #define xfree ruby_xfree
        busterarm 10y ago
        That's actually fascinating. I wonder if anyone has background on this...any comments I see are in Japanese.
        eudox 10y ago
        Replacing malloc/calloc/free etc an application-specific xmalloc/xcalloc/xfree is seemingly a fairly common pattern. There's one example in libiberty[0] (-liberty, get it?) where, for instance, the return value of malloc is checked for NULL and the program terminates if so. [0]: http://www.delorie.com/gnu/docs/gcc/libiberty_5.html http://www.delorie.com/gnu/docs/gcc/libiberty_5.html
        makomk 10y ago
        It's fairly common to define malloc and free wrappers called xmalloc and xfree. The usual idiomatic thing to do is to have xmalloc check the return value of malloc and terminate the program if it ever fails, though this is traditionally done in application code rather than a library. I doubt libc would ever add an xmalloc function because it'd break all the code that defined its own xmalloc.
asveikau 10y ago
The shortest code you can get away with is not always the best. If the parameter is a structure (which I think it is here, behind the void pointer), it's a good idea to give it its own free function in case later you add additional struct members that will need freeing/cleanup in the same place. (Bonus points if it appears in roughly the same place this thing is malloc'd) I would have been even more explicit and added a cast to the expected structure pointer type. Entirely useless, except to the human reader.
- userbinator 10y ago
  That sounds like premature generalisation to me. I'd say use a separate free function when it needs one, which is not the case yet (and might never be.)
  - asveikau 10y ago
    Call it an overreaction to a past project in which a predecessor scattered free calls over several uses, then I had to add fields. I don't mind adding 1 useless function per complex type if it saves me those headaches even a small minority of the time, or for the next maintainer. Opinions may differ, but that's me.
  - gpvos 10y ago
    I think this is more of a maintainability/readability case. Your suggestion sounds like premature optimization to me.

by bru 10y ago

busterarm 10y ago

C is not really my area of expertise, but doesn't xfree have alternatives? like free, freeif, etc? I'd much rather have to change that later in one place than several (if there end up being more calls to xfree in the code) if at some point the implementation needs to change again.

simcop2387 10y ago
So a from a quick googling it looks like xfree()[1] is a ruby specific thing, and should be used internally for everything allocated by ruby. It also looks like it's got the same signature as the function created for this so that should work there too. [2] [1] http://clalance.blogspot.com/2011/01/writing-ruby-extensions-in-c-part-12.html http://clalance.blogspot.com/2011/01/writing-ruby-extensions... [2] http://inferior-products.com/docs/userdocs/ruby19/html/d8/d16/gc_8c.html#a0bffec5b2cc004adcebb6802e7620387 http://inferior-products.com/docs/userdocs/ruby19/html/d8/d1...
- busterarm 10y ago
  ...Are you sure? I was pretty sure that it gets it from the same place as xmalloc...wouldn't that be in libc?
  - mrud 10y ago
    There is no xmalloc in the libc.
    - busterarm 10y ago
      Okay, interesting. I guess I've seen it before, so I didn't think it was Ruby-specific, but I also haven't had to do that kind of programming in a very super long time. Publib then? I'm finding this when I search: http://man.cx/xfree(3) http://man.cx/xfree(3) http://man.cx/publib(3) http://man.cx/publib(3) Also, are you sure? I see xmalloc in glibc all over the internet...
      - kristianp 10y ago
        edit: actually xmalloc and xfree are defined as #define xmalloc ruby_xmalloc #define xfree ruby_xfree
        busterarm 10y ago
        That's actually fascinating. I wonder if anyone has background on this...any comments I see are in Japanese.
        eudox 10y ago
        Replacing malloc/calloc/free etc an application-specific xmalloc/xcalloc/xfree is seemingly a fairly common pattern. There's one example in libiberty[0] (-liberty, get it?) where, for instance, the return value of malloc is checked for NULL and the program terminates if so. [0]: http://www.delorie.com/gnu/docs/gcc/libiberty_5.html http://www.delorie.com/gnu/docs/gcc/libiberty_5.html
      - makomk 10y ago
        It's fairly common to define malloc and free wrappers called xmalloc and xfree. The usual idiomatic thing to do is to have xmalloc check the return value of malloc and terminate the program if it ever fails, though this is traditionally done in application code rather than a library. I doubt libc would ever add an xmalloc function because it'd break all the code that defined its own xmalloc.

asveikau 10y ago

The shortest code you can get away with is not always the best. If the parameter is a structure (which I think it is here, behind the void pointer), it's a good idea to give it its own free function in case later you add additional struct members that will need freeing/cleanup in the same place. (Bonus points if it appears in roughly the same place this thing is malloc'd) I would have been even more explicit and added a cast to the expected structure pointer type. Entirely useless, except to the human reader.

userbinator 10y ago
That sounds like premature generalisation to me. I'd say use a separate free function when it needs one, which is not the case yet (and might never be.)
- asveikau 10y ago
  Call it an overreaction to a past project in which a predecessor scattered free calls over several uses, then I had to add fields. I don't mind adding 1 useless function per complex type if it saves me those headaches even a small minority of the time, or for the next maintainer. Opinions may differ, but that's me.
- gpvos 10y ago
  I think this is more of a maintainability/readability case. Your suggestion sounds like premature optimization to me.