Skip to content

Update: Memory leaks in stdup and asprintf#588

Merged
rangelmx merged 8 commits into
LinearTapeFileSystem:release/v2.4.8.4from
XV02:update/memory-leaks
May 15, 2026
Merged

Update: Memory leaks in stdup and asprintf#588
rangelmx merged 8 commits into
LinearTapeFileSystem:release/v2.4.8.4from
XV02:update/memory-leaks

Conversation

@XV02
Copy link
Copy Markdown
Contributor

@XV02 XV02 commented May 11, 2026

Summary of changes

This pull request includes following changes or fixes.

  • Fixes the unchanged memory allocations or return values from the functions strdup and asprintf.

Description

Most of strdups and asprintf functions had no checks on the success or failure of the action leading to potential errors, this PR adds just checks and returns errors according to the codebase section.

Type of change

  • Bug fix

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have confirmed my fix is effective or that my feature works

@XV02 XV02 self-assigned this May 11, 2026
@XV02 XV02 requested review from rangelmx and syaoraang May 12, 2026 14:51
Copy link
Copy Markdown
Collaborator

@syaoraang syaoraang left a comment

Choose a reason for hiding this comment

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

Seems pretty good changes, but still we need a few changes.

Comment thread src/tape_drivers/linux/lin_tape/lin_tape_ibmtape.c
Comment thread src/utils/ltfsck.c Outdated
Comment thread src/utils/ltfsck.c Outdated
Comment thread src/utils/mkltfs.c Outdated
Comment thread src/utils/mkltfs.c Outdated
Copy link
Copy Markdown
Member

@vandelvan vandelvan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution

@rangelmx
Copy link
Copy Markdown

LGTM!!
Thanks!

Just a question, do you have evidence of any tests performed?

Copy link
Copy Markdown
Collaborator

@syaoraang syaoraang left a comment

Choose a reason for hiding this comment

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

Excellent work. Thanks for addressing my requests!

Copy link
Copy Markdown
Contributor

@amissael95 amissael95 left a comment

Choose a reason for hiding this comment

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

If possible, we need to avoid the usage of "goto" for new changes. Since no loops are involved in this change we can keep as it is this time. Thanks for working on this change.

@rangelmx rangelmx merged commit 75a17a7 into LinearTapeFileSystem:release/v2.4.8.4 May 15, 2026
7 checks passed
@XV02
Copy link
Copy Markdown
Contributor Author

XV02 commented May 15, 2026

If possible, we need to avoid the usage of "goto" for new changes. Since no loops are involved in this change we can keep as it is this time. Thanks for working on this change.

Just for future reference, I used goto because C doesn't have a way of properly handling exceptions like other languages, goto statements used for cleanup is one of the few places where it's considered a good practice, as long as it stays local and doesn't jump upwards.

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.

6 participants