Skip to content

Conversation

rjd15372
Copy link
Member

@rjd15372 rjd15372 commented Oct 2, 2025

This commit introduces the support for modules to use the internal singly linked list implementation in their implementations.

This will be required by the future Lua scripting engine module.

New API functions:

  • ValkeyModule_ListCreate(): Create a new list with automatic memory management
  • ValkeyModule_ListFree(): Explicitly free a list
  • ValkeyModule_ListLength(): Get the number of elements in a list
  • ValkeyModule_ListAddToTail(): Add elements to the end of the list
  • ValkeyModule_ListGetIter(): Create an iterator for list traversal
  • ValkeyModule_ListIterNext(): Get the next element during iteration
  • ValkeyModule_ListReleaseIter(): Free the iterator

This commit introduces the support for modules to use the internal
singly linked list implementation in their implementations.

This will be required by the future Lua scripting engine module.

New API functions:
- `ValkeyModule_ListCreate()`: Create a new list with automatic memory management
- `ValkeyModule_ListFree()`: Explicitly free a list
- `ValkeyModule_ListLength()`: Get the number of elements in a list
- `ValkeyModule_ListAddToTail()`: Add elements to the end of the list
- `ValkeyModule_ListGetIter()`: Create an iterator for list traversal
- `ValkeyModule_ListIterNext()`: Get the next element during iteration
- `ValkeyModule_ListReleaseIter()`: Free the iterator

Signed-off-by: Ricardo Dias <[email protected]>
@rjd15372 rjd15372 added the needs-review Maintainer attention label Oct 2, 2025
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 20.00000% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.17%. Comparing base (3073979) to head (84a2ee0).

Files with missing lines Patch % Lines
src/module.c 20.00% 28 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2675      +/-   ##
============================================
- Coverage     72.18%   72.17%   -0.01%     
============================================
  Files           128      128              
  Lines         70994    71029      +35     
============================================
+ Hits          51246    51268      +22     
- Misses        19748    19761      +13     
Files with missing lines Coverage Δ
src/module.c 9.85% <20.00%> (+0.06%) ⬆️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ranshid
Copy link
Member

ranshid commented Oct 5, 2025

@rjd15372 trying to understand the motivation better. Is this because the standard C does not include any existing list implementation or is it intended for the auto memory collecting during context destruction?

@rjd15372
Copy link
Member Author

rjd15372 commented Oct 6, 2025

@rjd15372 trying to understand the motivation better. Is this because the standard C does not include any existing list implementation or is it intended for the auto memory collecting during context destruction?

The main reason is the former. Without this PR I would need to add a simple linked list implementation to the lua module. Which I don't strongly opposed if folks don't like having this API in the module API.

@zuiderkwast
Copy link
Contributor

It's nice to provide convenience datastructures for modules to use. We already have the module Dict (which is actually a radix tree) that modules can use.

I have some concerns though:

  1. This API doesn't look like a complete doubly-linked list API. For example, functions to push, pop and peek at the elements in both ends are not provided. Only push to tail.

  2. These new functions share the List prefix with, and can be confused with, the existing functions under the Key API for List type section (corresponding to valkey commands like LPUSH, etc.):

           int ValkeyModule_ListPush(ValkeyModuleKey *key,
                                     int where,
                                     ValkeyModuleString *ele);
    
           ValkeyModuleString *ValkeyModule_ListPop(ValkeyModuleKey *key, int where);
    
           ValkeyModuleString *ValkeyModule_ListGet(ValkeyModuleKey *key, long index);
    
           int ValkeyModule_ListSet(ValkeyModuleKey *key,
                                    long index,
                                    ValkeyModuleString *value);
    
           int ValkeyModule_ListInsert(ValkeyModuleKey *key,
                                       long index,
                                       ValkeyModuleString *value);
    
           int ValkeyModule_ListDelete(ValkeyModuleKey *key, long index);
    

With this in mind, I think it's better to start with an implementation of a linked list in the Lua module itself.

What more is needed for the Lua module?

@rjd15372
Copy link
Member Author

rjd15372 commented Oct 6, 2025

| What more is needed for the Lua module?

There are a few other changes to the module API, for which I'll open separate PRs to make it easy to review and discuss.

@zuiderkwast
Copy link
Contributor

| What more is needed for the Lua module?

There are a few other changes to the module API, for which I'll open separate PRs to make it easy to review and discuss.

OK, but it's good to have an overview so we can see the full picture. Is it more internal datatypes or other kinds of module API changes?

@rjd15372
Copy link
Member Author

rjd15372 commented Oct 6, 2025

it's just three new API functions:

VALKEYMODULE_API int (*ValkeyModule_ACLCheckCommandPermissionsForContextUser)(ValkeyModuleCtx *ctx,
                                                                              ValkeyModuleString **argv,
                                                                              int argc) VALKEYMODULE_ATTR;

VALKEYMODULE_API ValkeyModuleCallReply *(*ValkeyModule_CallArgv)(ValkeyModuleCtx *ctx,
                                                                 ValkeyModuleString **argv,
                                                                 size_t argc,
                                                                 ValkeyModuleCallCommandFlags flags)VALKEYMODULE_ATTR;

VALKEYMODULE_API int (*ValkeyModule_ReplyWithCustomErrorFormat)(ValkeyModuleCtx *ctx, int update_error_stats, const char *fmt, ...) VALKEYMODULE_ATTR;

@zuiderkwast zuiderkwast removed the needs-review Maintainer attention label Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants