Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce LZ4_STATIC_LINKING_ONLY_DISABLE_MEMORY_ALLOCATION #1124

Merged
merged 2 commits into from
Aug 5, 2022

Conversation

t-mat
Copy link
Contributor

@t-mat t-mat commented Jul 31, 2022

Part of #1071.

This changeset introduces new compile time switch macro LZ4_STATIC_LINKING_ONLY_DISABLE_MEMORY_ALLOCATION
which removes the following functions when it's defined.

// lz4.c
LZ4_createStream
LZ4_freeStream
LZ4_createStreamDecode
LZ4_freeStreamDecode
LZ4_create              // legacy

// lz4hc.c
LZ4_createStreamHC(void)
LZ4_freeStreamHC
LZ4_createHC            // legacy
LZ4_freeHC              // legacy

These functions uses dynamic memory allocation functions such as malloc() and free().
It'll be useful for freestanding environment which doesn't have these allocation functions.

Since this change breaks API, this macro is only valid with lz4 as a static linked object.

This changeset introduces new compile time switch macro LZ4_STATIC_LINKING_ONLY_DISABLE_MEMORY_ALLOCATION
which removes the following functions when it's defined.

```
// lz4.c
LZ4_createStream
LZ4_freeStream
LZ4_createStreamDecode
LZ4_freeStreamDecode
LZ4_create              // legacy

// lz4hc.c
LZ4_createStreamHC(void)
LZ4_freeStreamHC
LZ4_createHC            // legacy
LZ4_freeHC              // legacy
```

These functions uses dynamic memory allocation functions such as malloc() and free().
It'll be useful for freestanding environment which doesn't have these allocation functions.

Since this change breaks API, this macro is only valid with lz4 as a static linked object.
lib/lz4.c Outdated
# undef LZ4_ALLOC
# undef LZ4_ALLOC_AND_ZERO
# undef LZ4_FREEMEM
# define LZ4_ALLOC(x) lz4_error_memory_allocation_is_disabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand properly, these macros LZ4_ALLOC(x) and next are designed to fail at compile time, as a way to detect if they are employed anywhere.

However, these macros are also not used anywhere.

Did you mean to replace ALLOC(s) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops. You're right. They must be ALLOC(s), ALLOC_AND_ZERO(s) and FREEMEM(p).

@@ -188,7 +188,11 @@
/*-************************************
* Memory routines
**************************************/
#ifdef LZ4_USER_MEMORY_FUNCTIONS
#if defined(LZ4_STATIC_LINKING_ONLY_DISABLE_MEMORY_ALLOCATION)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will users be made aware of this new capability ?
Maybe it should be documented ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! It needs document. Draft version:

/*!
 * LZ4_STATIC_LINKING_ONLY_DISABLE_MEMORY_ALLOCATION :
 * Disable relatively high-level LZ4/HC functions which uses dynamic memory
 * allocation (malloc(), calloc(), free()).
 *
 * Note that this is a compile-time switch.  And since it disables public/stable
 * LZ4 v1 API functions, we don't recommend to use this symbol to generate
 * library for distribution.
 *
 * The following functions are removed when this symbol is defined.
 *
 * // lz4.c
 * LZ4_createStream
 * LZ4_freeStream
 * LZ4_createStreamDecode
 * LZ4_freeStreamDecode
 * LZ4_create              // legacy
 * 
 * // lz4hc.c
 * LZ4_createStreamHC(void)
 * LZ4_freeStreamHC
 * LZ4_createHC            // legacy
 * LZ4_freeHC              // legacy
 */

I also realize I should add #ifndef to declaration of these functions in header.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you expect to provide this documentation as part of this PR ?
Or do you want to make it a separate PR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to send it as a separate PR. Sorry!

@Cyan4973 Cyan4973 merged commit ca26930 into lz4:dev Aug 5, 2022
@t-mat t-mat deleted the compile-time-purge-memalloc-func branch August 6, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants