Re: [PATCH v3 13/15] unzstd: replace INIT{,DATA} and STATIC
Hi Jan, On 15/04/2021 15:34, Jan Beulich wrote: On 15.04.2021 16:30, Julien Grall wrote: On 15/04/2021 15:25, Jan Beulich wrote: On 15.04.2021 16:22, Julien Grall wrote: On 15/04/2021 15:21, Jan Beulich wrote: On 15.04.2021 13:59, Julien Grall wrote: On 26/01/2021 09:52, Jan Beulich wrote: With xen/common/decompress.h now agreeing in both build modes about what STATIC expands to, there's no need for this abstraction anymore. Shouldn't you also mention "INIT" and "INITDATA" here? Two parts: INITDATA was mistakenly mentioned in the title. I've dropped that. Ok. And what I'm saying about STATIC does not apply to INIT - for it, we replace the extra level of abstraction by directly using __init, just like was done in the earlier patches. This should be mention in the commit message. It already is by what is being said after the comma. May I direct you back to the commit messages of earlier patches in this series (when talk was of just INIT)? From the way the commit message is written it sounds like more you are referring to STATIC only. This is a clearer on the other commit messages because there is no other way to interpret "this". If the commit message is taken together with the title, I think all is clear. I have to admit, I view the commit title as a one line summary. So I often read them separately. So I would suggest to clarify it. And I would like to avoid doing so. As a compromise, I'll change "this abstraction" to "these abstractions". Sounds good to me. Cheers, -- Julien Grall
Re: [PATCH v3 13/15] unzstd: replace INIT{,DATA} and STATIC
On 15.04.2021 16:30, Julien Grall wrote: > On 15/04/2021 15:25, Jan Beulich wrote: >> On 15.04.2021 16:22, Julien Grall wrote: >>> On 15/04/2021 15:21, Jan Beulich wrote: On 15.04.2021 13:59, Julien Grall wrote: > On 26/01/2021 09:52, Jan Beulich wrote: >> With xen/common/decompress.h now agreeing in both build modes about >> what STATIC expands to, there's no need for this abstraction anymore. > > Shouldn't you also mention "INIT" and "INITDATA" here? Two parts: INITDATA was mistakenly mentioned in the title. I've dropped that. >>> >>> Ok. >>> >>> And what I'm saying about STATIC does not apply to INIT - for it, we replace the extra level of abstraction by directly using __init, just like was done in the earlier patches. >>> >>> This should be mention in the commit message. >> >> It already is by what is being said after the comma. May I direct >> you back to the commit messages of earlier patches in this series >> (when talk was of just INIT)? > From the way the commit message is written it sounds like more you are > referring to STATIC only. This is a clearer on the other commit messages > because there is no other way to interpret "this". If the commit message is taken together with the title, I think all is clear. > So I would suggest to clarify it. And I would like to avoid doing so. As a compromise, I'll change "this abstraction" to "these abstractions". Jan
Re: [PATCH v3 13/15] unzstd: replace INIT{,DATA} and STATIC
On 15/04/2021 15:25, Jan Beulich wrote: On 15.04.2021 16:22, Julien Grall wrote: On 15/04/2021 15:21, Jan Beulich wrote: On 15.04.2021 13:59, Julien Grall wrote: On 26/01/2021 09:52, Jan Beulich wrote: With xen/common/decompress.h now agreeing in both build modes about what STATIC expands to, there's no need for this abstraction anymore. Shouldn't you also mention "INIT" and "INITDATA" here? Two parts: INITDATA was mistakenly mentioned in the title. I've dropped that. Ok. And what I'm saying about STATIC does not apply to INIT - for it, we replace the extra level of abstraction by directly using __init, just like was done in the earlier patches. This should be mention in the commit message. It already is by what is being said after the comma. May I direct you back to the commit messages of earlier patches in this series (when talk was of just INIT)? From the way the commit message is written it sounds like more you are referring to STATIC only. This is a clearer on the other commit messages because there is no other way to interpret "this". So I would suggest to clarify it. Cheers, -- Julien Grall
Re: [PATCH v3 13/15] unzstd: replace INIT{,DATA} and STATIC
On 15.04.2021 16:22, Julien Grall wrote: > On 15/04/2021 15:21, Jan Beulich wrote: >> On 15.04.2021 13:59, Julien Grall wrote: >>> On 26/01/2021 09:52, Jan Beulich wrote: With xen/common/decompress.h now agreeing in both build modes about what STATIC expands to, there's no need for this abstraction anymore. >>> >>> Shouldn't you also mention "INIT" and "INITDATA" here? >> >> Two parts: INITDATA was mistakenly mentioned in the title. I've >> dropped that. > > Ok. > > And what I'm saying about STATIC does not apply to >> INIT - for it, we replace the extra level of abstraction by >> directly using __init, just like was done in the earlier patches. > > This should be mention in the commit message. It already is by what is being said after the comma. May I direct you back to the commit messages of earlier patches in this series (when talk was of just INIT)? Jan
Re: [PATCH v3 13/15] unzstd: replace INIT{,DATA} and STATIC
On 15/04/2021 15:21, Jan Beulich wrote: On 15.04.2021 13:59, Julien Grall wrote: On 26/01/2021 09:52, Jan Beulich wrote: With xen/common/decompress.h now agreeing in both build modes about what STATIC expands to, there's no need for this abstraction anymore. Shouldn't you also mention "INIT" and "INITDATA" here? Two parts: INITDATA was mistakenly mentioned in the title. I've dropped that. Ok. And what I'm saying about STATIC does not apply to INIT - for it, we replace the extra level of abstraction by directly using __init, just like was done in the earlier patches. This should be mention in the commit message. Cheers, -- Julien Grall
Re: [PATCH v3 13/15] unzstd: replace INIT{,DATA} and STATIC
On 15.04.2021 13:59, Julien Grall wrote: > On 26/01/2021 09:52, Jan Beulich wrote: >> With xen/common/decompress.h now agreeing in both build modes about >> what STATIC expands to, there's no need for this abstraction anymore. > > Shouldn't you also mention "INIT" and "INITDATA" here? Two parts: INITDATA was mistakenly mentioned in the title. I've dropped that. And what I'm saying about STATIC does not apply to INIT - for it, we replace the extra level of abstraction by directly using __init, just like was done in the earlier patches. Jan
Re: [PATCH v3 13/15] unzstd: replace INIT{,DATA} and STATIC
Hi Jan,
On 26/01/2021 09:52, Jan Beulich wrote:
With xen/common/decompress.h now agreeing in both build modes about
what STATIC expands to, there's no need for this abstraction anymore.
Shouldn't you also mention "INIT" and "INITDATA" here?
Cheers,
Requested-by: Andrew Cooper
Signed-off-by: Jan Beulich
---
v3: New.
--- a/xen/common/unzstd.c
+++ b/xen/common/unzstd.c
@@ -71,7 +71,7 @@
*/
#define ZSTD_IOBUF_SIZE (1 << 17)
-static int INIT handle_zstd_error(size_t ret, void (*error)(const char *x))
+static int __init handle_zstd_error(size_t ret, void (*error)(const char *x))
{
const int err = ZSTD_getErrorCode(ret);
@@ -102,9 +102,9 @@ static int INIT handle_zstd_error(size_t
* We can allocate less memory (no circular buffer for the sliding window),
* and avoid some memcpy() calls.
*/
-static int INIT decompress_single(const u8 *in_buf, long in_len, u8 *out_buf,
- long out_len, unsigned int *in_pos,
- void (*error)(const char *x))
+static int __init decompress_single(const u8 *in_buf, long in_len, u8 *out_buf,
+ long out_len, unsigned int *in_pos,
+ void (*error)(const char *x))
{
const size_t wksp_size = ZSTD_DCtxWorkspaceBound();
void *wksp = large_malloc(wksp_size);
@@ -142,11 +142,11 @@ out:
return err;
}
-int INIT unzstd(unsigned char *in_buf, unsigned int in_len,
- int (*fill)(void*, unsigned int),
- int (*flush)(void*, unsigned int),
- unsigned char *out_buf, unsigned int *in_pos,
- void (*error)(const char *x))
+int __init unzstd(unsigned char *in_buf, unsigned int in_len,
+ int (*fill)(void*, unsigned int),
+ int (*flush)(void*, unsigned int),
+ unsigned char *out_buf, unsigned int *in_pos,
+ void (*error)(const char *x))
{
ZSTD_inBuffer in;
ZSTD_outBuffer out;
--- a/xen/common/zstd/decompress.c
+++ b/xen/common/zstd/decompress.c
@@ -46,7 +46,7 @@
/*_***
* Memory operations
**/
-static void INIT ZSTD_copy4(void *dst, const void *src) { memcpy(dst, src, 4);
}
+static void __init ZSTD_copy4(void *dst, const void *src) { memcpy(dst, src,
4); }
/*-*
* Context management
@@ -98,12 +98,12 @@ struct ZSTD_DCtx_s {
BYTE headerBuffer[ZSTD_FRAMEHEADERSIZE_MAX];
}; /* typedef'd to ZSTD_DCtx within "zstd.h" */
-STATIC size_t INIT ZSTD_DCtxWorkspaceBound(void)
+static size_t __init ZSTD_DCtxWorkspaceBound(void)
{
return ZSTD_ALIGN(sizeof(ZSTD_stack)) + ZSTD_ALIGN(sizeof(ZSTD_DCtx));
}
-STATIC size_t INIT ZSTD_decompressBegin(ZSTD_DCtx *dctx)
+static size_t __init ZSTD_decompressBegin(ZSTD_DCtx *dctx)
{
dctx->expected = ZSTD_frameHeaderSize_prefix;
dctx->stage = ZSTDds_getFrameHeaderSize;
@@ -123,7 +123,7 @@ STATIC size_t INIT ZSTD_decompressBegin(
return 0;
}
-STATIC ZSTD_DCtx *INIT ZSTD_createDCtx_advanced(ZSTD_customMem customMem)
+static ZSTD_DCtx *__init ZSTD_createDCtx_advanced(ZSTD_customMem customMem)
{
ZSTD_DCtx *dctx;
@@ -138,13 +138,13 @@ STATIC ZSTD_DCtx *INIT ZSTD_createDCtx_a
return dctx;
}
-STATIC ZSTD_DCtx *INIT ZSTD_initDCtx(void *workspace, size_t workspaceSize)
+static ZSTD_DCtx *__init ZSTD_initDCtx(void *workspace, size_t workspaceSize)
{
ZSTD_customMem const stackMem = ZSTD_initStack(workspace,
workspaceSize);
return ZSTD_createDCtx_advanced(stackMem);
}
-size_t INIT ZSTD_freeDCtx(ZSTD_DCtx *dctx)
+size_t __init ZSTD_freeDCtx(ZSTD_DCtx *dctx)
{
if (dctx == NULL)
return 0; /* support free on NULL */
@@ -153,15 +153,15 @@ size_t INIT ZSTD_freeDCtx(ZSTD_DCtx *dct
}
#ifdef BUILD_DEAD_CODE
-void INIT ZSTD_copyDCtx(ZSTD_DCtx *dstDCtx, const ZSTD_DCtx *srcDCtx)
+void __init ZSTD_copyDCtx(ZSTD_DCtx *dstDCtx, const ZSTD_DCtx *srcDCtx)
{
size_t const workSpaceSize = (ZSTD_BLOCKSIZE_ABSOLUTEMAX +
WILDCOPY_OVERLENGTH) + ZSTD_frameHeaderSize_max;
memcpy(dstDCtx, srcDCtx, sizeof(ZSTD_DCtx) - workSpaceSize); /* no need
to copy workspace */
}
#endif
-STATIC size_t ZSTD_findFrameCompressedSize(const void *src, size_t srcSize);
-STATIC size_t ZSTD_decompressBegin_usingDict(ZSTD_DCtx *dctx, const void *dict,
+static size_t ZSTD_findFrameCompressedSize(const void *src, size_t srcSize);
+static size_t ZSTD_decompressBegin_usingDict(ZSTD_DCtx *dctx, const void *dict,
size_t dictSize);
static void ZSTD_refDDict(ZSTD_DCtx *dstDCtx, const ZSTD_DDict *ddict);
@@ -176,7 +176,7 @@ static void ZSTD_refDDict(ZSTD_DCtx *dst
* Note : Frame Identifier is 4 bytes. If `size < 4`, @return will
