[NEW] Rename zmalloc (and all z-alloc functions) valkey_zmalloc #1157
Comments
|
OK, interesting. I haven't seen this before. We do link OpenSSL statically and we don't know how that external OpenSSL was built, so I think we should accept that it may have been built with zlib. Changing all calls to zmalloc function would be a big change, but if we rename the functions and add |
|
Yes for us the I personally find it ugly, and in the projects I do maintain myself, I would go for the mass renaming (and I do understand this comes at the cost of creating more conflicts with downstream forks, it seems now redis seems to pull changes from valkey). At the end, maintainers of valkey are the decision makers, if you use only the To have the problem, you need to link against a static openssl which also uses a static libz, so disable the plugin machinery of openssl to favor embedded libraries instead. I think this is not the default, but for people willing to deliver binaries as standalone as possible, this is what some are doing. |
|
Yeah, I would accept the |
|
I can submit a pull request for that. You want really just the more minimal |
|
I'd say the minimal #define solution to start with, let's say for zmalloc, zcalloc, zrealloc, zfree. I don't know if we need if for all the variants like I don't know if the others in the core team agree, so I suggest making a small PR and get some feedback first. |
…oc,free} The zcalloc symbol is a symbol name already used by zlib, which is defining other names using the "z" prefix specific to zlib. Valkey has historically defined the zcalloc symbol as well, next to other z-alloc derivatives, while these symbols should have used the "valkey_" prefix specific to valkey, to avoid name collisions. In practice, linking valkey with a static openssl, which itself might depend on a static libz will result in link time error rejecting multiple zcalloc symbol definitions. Ideally, in the long term, the z-definitions used in valkey shall instead be massively renamed using the "valkey_". It was agreed initially to just go for the easy way to rename definitions, via couple of #define renames. Fixes: valkey-io#1157
…oc,free} The zcalloc symbol is a symbol name already used by zlib, which is defining other names using the "z" prefix specific to zlib. Valkey has historically defined the zcalloc symbol as well, next to other z-alloc derivatives, while these symbols should have used the "valkey_" prefix specific to valkey, to avoid name collisions. In practice, linking valkey with a static openssl, which itself might depend on a static libz will result in link time error rejecting multiple zcalloc symbol definitions. Ideally, in the long term, the z-definitions used in valkey shall instead be massively renamed using the "valkey_". It was agreed initially to just go for the easy way to rename definitions, via couple of #define renames. Fixes: valkey-io#1157 Signed-off-by: Romain Geissler <romain.geissler@amadeus.com>
…oc,free} The zcalloc symbol is a symbol name already used by zlib, which is defining other names using the "z" prefix specific to zlib. Valkey has historically defined the zcalloc symbol as well, next to other z-alloc derivatives, while these symbols should have used the "valkey_" prefix specific to valkey, to avoid name collisions. In practice, linking valkey with a static openssl, which itself might depend on a static libz will result in link time error rejecting multiple zcalloc symbol definitions. Ideally, in the long term, the z-definitions used in valkey shall instead be massively renamed using the "valkey_". It was agreed initially to just go for the easy way to rename definitions, via couple of #define renames. Fixes: valkey-io#1157 Signed-off-by: Romain Geissler <romain.geissler@amadeus.com>
…oc,free} The zcalloc symbol is a symbol name already used by zlib, which is defining other names using the "z" prefix specific to zlib. In practice, linking valkey with a static openssl, which itself might depend on a static libz will result in link time error rejecting multiple symbol definitions. Fixes: valkey-io#1157 Signed-off-by: Romain Geissler <romain.geissler@amadeus.com>
…oc,free} (valkey-io#1169) The zcalloc symbol is a symbol name already used by zlib, which is defining other names using the "z" prefix specific to zlib. In practice, linking valkey with a static openssl, which itself might depend on a static libz will result in link time error rejecting multiple symbol definitions. Fixes: valkey-io#1157 Signed-off-by: Romain Geissler <romain.geissler@amadeus.com>
The problem/use-case that the feature addresses
This is a re-opening of an issue we raised on redis years ago. It was redis/redis#9986
I tried the latest redis server and it still suffer the same issue with redis 7.4.1. I didn't try valkey, but given that the issue was there years ago, and that the valkey fork is rather recent, you shall suffer the same issue too. As soon as you try to create a binary with a static openssl and a static zlib, you end up with some symbol collision, at least for zcalloc.
A suggested back then, it would be cool of these z-alloc function would be namespaced. In the valkey project, this would typically be named valkey_zmalloc.
Alternatives you've considered
Currently when we build redis (it would be the same with valkey), we patch redis sources with an ugly
#define zcalloc redis_zcallocand it works. However it's no more than ugly workaround, for me this shall be fixed directly in the source code via a massive renaming. The redis team told us back then they are against massive renaming, but given you have massively replaced redis by valkey, I doubt you have the same ideological barriers ;)The text was updated successfully, but these errors were encountered: