Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Embrace, extend, extinguish.

Does Redis still do that thing where it forks and the child writes its core to disk? How does that work under Windows, which doesn't have fork?

Finally, this is one big patch:

    339 files changed, 146821 insertions(+), 290 deletions(-)
With many of the changes along the lines of:

     static void *callbackValDup(void *privdata, const void *src) {
    -    ((void) privdata);
         redisCallback *dup = malloc(sizeof(*dup));
    +    ((void) privdata);
         memcpy(dup,src,sizeof(*dup));
         return dup;
     }
or

    -    cmd = malloc(totlen+1);
    +    cmd = (char*)malloc(totlen+1);
Eliminating compiler warnings is nice and whatever, but probably not the best thing to include in your "add major feature" patch.


Not sure what they're going to do regarding fork, but they say this at the end of the gist:

TODO

    Snapshotting (Fork and Write) is not perfect, right now we simply block requests while memory is dumped on disk. We are working on a solution that will give us better performance. An update will be released soon.


So this 146821 line monstrosity isn't even theoretically usable?

(You know, I could rewrite Redis in less than 146,000 lines of code...)


I wonder if they contacted Redis author before starting to work on this. You know, with the patch so big and radical, there's a possibility he doesn't even want to accept it.

What then, all this effort for basically nothing except a fork, which you then have to continue maintaining etc.


I'd be surprised if he accepted a patch that added a dep; Salvatore seems allergic to deps (something I like about Redis).


(on a mobile and fat-fingered a downvote, sorry)

I wouldn't expect anyone to accept the patch in this state, but I hope that the "who cares about Windows" attitude dies and a dialog to get proper support into redis is started.

It's a shame that we're using an unofficial version on that platform right now.


You don't have to have a "who cares about Windows?" attitude to write programs that don't work on Windows; all you have to have is a "want to make the best use of Unix" attitude.


Correct me if I'm wrong, but casting the result of malloc is generally considered bad form in C, isn't it?


In general, yes. For Win32, it is safe.

I would imagine they have activated "Compile as C++" for certain files. VC++ doesn't support C99, and it's probably easier to fix up any C++ incompatibilities than try to C89-ize everything.


http://stackoverflow.com/questions/605845/do-i-cast-the-resu... is a good place to start for a deeper discussion of this topic, if someone is interested.

My answer (obviously) is "yes", although as some folks point out here too, there are portability concerns that make it worth it sometimes.


I'm just assuming it's a Windowsism.


I wouldn't so much call it "bad form" as just non-idiomatic. It doesn't really have big downsides in practice.

It's mostly a habit people pick up from C++ (where it's mandatory due to stricter typing), and if you want to build C code with a C++ compiler, you need to add the casts.

Given Microsoft's C++ fetish, I'm unsurprised by this.


I suppose one downside would be if you cast a voidptr to a pointer to (an element that has a different size), then calling operator++ moves it by more than if you had left it as a voidptr.


There are 453 mentions of "fork" in the patch, so I'm imagining it does.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: