Skip to content

4.1.11 #53

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.

Sign up for GitHub

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

Merged
mscasso-scanoss merged 5 commits into from
Mar 28, 2026
Merged

4.1.11 #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc59aee
fix bug removing extra records with collate
mscasso-scanoss Mar 25, 2026
6a10d61
fix bug deleting specific records
mscasso-scanoss Mar 25, 2026
e33d3f8
fix memory allocation on collate
mscasso-scanoss Mar 28, 2026
c81da26
fix log typo
mscasso-scanoss Mar 28, 2026
5d953b6
fix bug in delete specific records
mscasso-scanoss Mar 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions src/collate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,15 @@ bool ldb_import_list_variable_records(struct ldb_collate_data *collate)
uint8_t *last_data = calloc(collate->rec_width, 1);
uint16_t last_rec_size = 0;

if (!buffer || !last_key || !last_data)
{
free(buffer);
free(last_key);
free(last_data);
fprintf(stderr, "Memory allocation failed in ldb_import_list_variable_records\n");
return false;
}
Comment on lines +182 to +189

@coderabbitai coderabbitai Bot Mar 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Unify all failure exits through one cleanup path to prevent leaks.

On Line 221 and Line 269, returning directly skips frees for buffer, last_key, and last_data. Route all error exits to a shared cleanup label.

♻️ Suggested refactor 
 bool ldb_import_list_variable_records(struct ldb_collate_data *collate)
 {
@@
 	if (!buffer || !last_key || !last_data)
 	{
-		free(buffer);
-		free(last_key);
-		free(last_data);
-		log_info("Memory allocation failed in ldb_import_list_variable_records\n");
-		return false;
+		log_info("Memory allocation failed in ldb_import_list_variable_records\n");
+		goto cleanup_fail;
 	}
@@
 				int error =ldb_node_write(out_table, new_sector, last_key, buffer, buffer_ptr, 0);
 				//abort in case of error
 				if (error < 0)
-					return false;
+					goto cleanup_fail;
@@
 		int error= ldb_node_write(out_table, new_sector, last_key, buffer, buffer_ptr, 0);
 		//abort in case of error
 		if (error < 0)
-			return false;
+			goto cleanup_fail;
 	}
@@
 	free(buffer);
 	free(last_key);
 	free(last_data);
 
 	return true;
+
+cleanup_fail:
+	free(buffer);
+	free(last_key);
+	free(last_data);
+	return false;
 }

Also applies to: 218-221, 266-269

🧰 Tools
🪛 GitHub Actions: build

[warning] 187-187: gcc warning: implicit declaration of function 'log_error'; did you mean 'ldb_error'? [-Wimplicit-function-declaration]

[error] 187-187: Link failed (ld): undefined reference to 'log_error' from 'ldb_import_list_variable_records' during building target 'ldb'. make: *** [Makefile:12: ldb] Error 1

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/collate.c` around lines 182 - 189, Unify all error exits in
ldb_import_list_variable_records by introducing a single cleanup label (e.g.,
cleanup or error_cleanup) and replacing direct returns on allocation/check
failures with goto to that label; ensure the cleanup block frees buffer,
last_key, and last_data (if non-NULL), logs the error via log_error("Memory
allocation failed in ldb_import_list_variable_records\n") if appropriate, and
returns false. Update every early exit path that currently does free() then
return (including the places around the current checks for
buffer/last_key/last_data and other returns) to jump to the shared cleanup label
instead so no leak is possible. Ensure normal success return bypasses the
cleanup or performs any needed final free before returning true.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify logger symbols available in this repository (read-only).
fd -i 'logger.h'
rg -nP --type=c --type=h '\blog_error\s*\('
rg -nP --type=c --type=h '\b(log_info|log_debug|ldb_error)\s*\('

Repository: scanoss/ldb

Length of output: 16012

Replace undefined log_error with log_info to fix the build break.

Line 187 calls log_error, which is not declared or defined in this codebase. The logger API only provides log_info and log_debug (declared in src/logger.h). Replace with log_info:

-		log_error("Memory allocation failed in ldb_import_list_variable_records\n");
+		log_info("Memory allocation failed in ldb_import_list_variable_records\n");
🧰 Tools
🪛 GitHub Actions: build

[warning] 187-187: gcc warning: implicit declaration of function 'log_error'; did you mean 'ldb_error'? [-Wimplicit-function-declaration]

[error] 187-187: Link failed (ld): undefined reference to 'log_error' from 'ldb_import_list_variable_records' during building target 'ldb'. make: *** [Makefile:12: ldb] Error 1

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/collate.c` around lines 182 - 189, In ldb_import_list_variable_records,
the error path that checks if (!buffer || !last_key || !last_data) calls the
undefined log_error; replace that call with the available logger function
log_info (preserving the existing message), i.e. update the log_error("Memory
allocation failed in ldb_import_list_variable_records\n") invocation to use
log_info so the code builds against the logger API; ensure the function name
ldb_import_list_variable_records and the variables buffer, last_key, last_data
are left unchanged.


for (long data_ptr = 0; data_ptr < collate->data_ptr; data_ptr += collate->rec_width)
{
rec_key = collate->data + data_ptr;
Expand All @@ -188,17 +197,17 @@ bool ldb_import_list_variable_records(struct ldb_collate_data *collate)
if (collate->table_rec_ln) rec_size = collate->table_rec_ln;
else rec_size = uint32_read(rec_key + collate->rec_width - LDB_KEY_LN);

/* If record is duplicated, skip it */
if (rec_size == last_rec_size) if (!memcmp(data, last_data, rec_size)) continue;
/* Check if key is different than the last one */
new_subkey = (memcmp(rec_key+LDB_KEY_LN, last_key+LDB_KEY_LN, subkey_ln) != 0);

/* If record is duplicated (same subkey and same data), skip it */
if (!new_subkey && rec_size == last_rec_size && !memcmp(data, last_data, rec_size)) continue;
Comment on lines +200 to +204

@coderabbitai coderabbitai Bot Mar 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's explore the structure and locate the file
git ls-files | grep -E "collate\.(c|h)$"

Repository: scanoss/ldb

Length of output: 86

🏁 Script executed:

# Get the file size to understand how to read it
wc -l src/collate.c

Repository: scanoss/ldb

Length of output: 72

🏁 Script executed:

# Read lines 150-250 to get full context around lines 200-204 and 230
sed -n '150,250p' src/collate.c | cat -n

Repository: scanoss/ldb

Length of output: 3935

🏁 Script executed:

# Read the rest of the function to see line 230 and understand the complete logic
sed -n '100,250p' src/collate.c | cat -n

Repository: scanoss/ldb

Length of output: 5611

🏁 Script executed:

# Continue reading to see what happens after the new_subkey block
sed -n '230,280p' src/collate.c | cat -n

Repository: scanoss/ldb

Length of output: 1676

🏁 Script executed:

# Check if subkey_ln can be 0
grep -n "subkey_ln" src/collate.c | head -20

Repository: scanoss/ldb

Length of output: 1551

🏁 Script executed:

# Check the header to understand table_key_ln and LDB_KEY_LN
cat -n src/ldb/collate.h | head -50

Repository: scanoss/ldb

Length of output: 1962

🏁 Script executed:

# Check the definition of LDB_KEY_LN to understand the structure
grep -r "define LDB_KEY_LN" . --include="*.h"

Repository: scanoss/ldb

Length of output: 124

🏁 Script executed:

# Look for usage patterns to see if table_key_ln can equal LDB_KEY_LN (which would make subkey_ln == 0)
rg "table_key_ln\s*=" src/collate.c

Repository: scanoss/ldb

Length of output: 93

🏁 Script executed:

# Verify if memcmp with length 0 returns 0
python3 << 'EOF'
# Test what memcmp does with length 0
# In C, memcmp(ptr1, ptr2, 0) returns 0
# Let's confirm this is the expected behavior
print("memcmp behavior with length 0:")
print("memcmp(any, any, 0) always returns 0")
print("This means: (0 != 0) evaluates to false")
print()
print("So if subkey_ln == 0:")
print("  new_subkey = (memcmp(..., ..., 0) != 0)")
print("  new_subkey = (0 != 0)")
print("  new_subkey = false")
print()
print("On first iteration with rec_group_size == 0:")
print("The new-group block (lines 131-151) would be SKIPPED")
print("This is problematic because the group structure isn't initialized.")
EOF

Repository: scanoss/ldb

Length of output: 438

🏁 Script executed:

# Check the actual code flow to confirm the issue scenario
sed -n '200,260p' src/collate.c | cat -n

Repository: scanoss/ldb

Length of output: 2407

🏁 Script executed:

# Let's trace through the exact buffer write operations to confirm the issue
# Check how rec_group_start is used when writing the group size
rg "rec_group_start\s*\+" src/collate.c -A 1 -B 1

Repository: scanoss/ldb

Length of output: 540

Guard first-iteration group initialization when computing new_subkey.

At line 201, new_subkey is overwritten by memcmp result. When subkey_ln == 0 (which occurs if table_key_ln == LDB_KEY_LN), memcmp(..., ..., 0) returns 0, making new_subkey = false on the first record. This causes the new-group initialization block (lines 231-251) to be skipped, leaving rec_group_start at its initial value of 0 while buffer_ptr advances. Subsequent group size writes at buffer + rec_group_start + subkey_ln then target incorrect buffer locations, corrupting the output.

🛠️ Suggested fix 
-		new_subkey = (memcmp(rec_key+LDB_KEY_LN, last_key+LDB_KEY_LN, subkey_ln) != 0);
+		new_subkey = (rec_group_size == 0) ||
+		             (memcmp(rec_key + LDB_KEY_LN, last_key + LDB_KEY_LN, subkey_ln) != 0);


/* Update last record */
memcpy(last_data, data, rec_size);
last_rec_size = rec_size;

uint32_t projected_size = buffer_ptr + rec_size + collate->table_key_ln + (2 * LDB_PTR_LN) + out_table.ts_ln;

/* Check if key is different than the last one */
new_subkey = (memcmp(rec_key+LDB_KEY_LN, last_key+LDB_KEY_LN, subkey_ln) != 0);
/* If node size is exceeded, initialize buffer */
if (projected_size >= LDB_MAX_REC_LN)
{
Expand Down Expand Up @@ -442,6 +451,8 @@ static bool data_compare(char * a, char * b)
log_debug("<<<comparing: %s / %s : %d >>\n", buffer_a, buffer_b, r);
if (!skip_field && r)
return false;
if (!*a || !*b)
return (*a == *b);
a++;
b++;
memset(buffer_a, 0, LDB_MAX_REC_LN);
Expand All @@ -465,7 +476,7 @@ bool key_in_delete_list(struct ldb_collate_data *collate, uint8_t *key, uint8_t
{
/* Position pointer to start of second byte in the sorted del_key array */
for (int i = 0; i < collate->del_tuples->tuples_number; i++)
{
{
/*The keys are sorted, if I'm in another sector a must out*/
if (collate->del_tuples->tuples[i]->key[0] > key[0])
return false;
Expand All @@ -474,16 +485,16 @@ bool key_in_delete_list(struct ldb_collate_data *collate, uint8_t *key, uint8_t

/* First byte is always the same, second too inside this loop. Compare bytes 2, 3 and 4 */
int mainkey = memcmp(collate->del_tuples->tuples[i]->key + 1, key + 1, LDB_KEY_LN -1);
if (mainkey != 0)
if (mainkey != 0)
continue; //return false;

/*For fixed records there is no subkey, so key hex will be empty*/
char key_hex1[collate->in_table.key_ln * 2 + 1];
char key_hex2[collate->in_table.key_ln * 2 + 1];
ldb_bin_to_hex(subkey, subkey_ln, key_hex1);
ldb_bin_to_hex(&collate->del_tuples->tuples[i]->key[LDB_KEY_LN], subkey_ln, key_hex2);
/*Math the rest of the key*/

/*Match the rest of the key*/
if (!memcmp(subkey, &collate->del_tuples->tuples[i]->key[LDB_KEY_LN], subkey_ln))
{
bool result = true;
Expand Down
2 changes: 1 addition & 1 deletion src/ldb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include "./ldb/types.h"
#include "./ldb/mz.h"

#define LDB_VERSION "4.1.9"
#define LDB_VERSION "4.1.11"

#define LDB_TABLE_DEFINITION_UNDEFINED -1
#define LDB_TABLE_DEFINITION_STANDARD 0
Expand Down
Loading