Skip to content

Comments

Fix: missing cleanup on fail in print_report_xml_start#2668

Merged
greenbonebot merged 1 commit intomainfrom
smaller-print-report-6
Dec 8, 2025
Merged

Fix: missing cleanup on fail in print_report_xml_start#2668
greenbonebot merged 1 commit intomainfrom
smaller-print-report-6

Conversation

@mattmundell
Copy link
Contributor

@mattmundell mattmundell commented Dec 5, 2025

What

Use a simple fail jump instead of trying to do the cleanup at every return.

Why

This catches a few error exits that were not calling tz_revert and/or where not closing "out". Having so many separate returns was error prone.

Part of reducing the size of print_report_xml_start.

References

Follows /pull/2662.

Testing

I got a single report with the PR, then got the same report on the main branch. I made sure they were the same. I did that with two timezones just to be sure (using the timezone term in the filter).

o m m '<get_reports report_id="92a09ffd-a5bd-464b-9f2c-54b78ec5ebcf" details="1" filter="timezone=Pacific/Auckland rows=3 apply_overrides=1 min_qod=60"/>' > /tmp/r-auk-main
o m m '<get_reports report_id="92a09ffd-a5bd-464b-9f2c-54b78ec5ebcf" details="1" filter="timezone=Europe/Berlin rows=3 apply_overrides=1 min_qod=60"/>' > /tmp/r-ber-main
diff /tmp/r-auk /tmp/r-auk-main
diff /tmp/r-ber /tmp/r-ber-main

@mattmundell mattmundell requested review from a team as code owners December 5, 2025 14:07
@greenbonebot greenbonebot enabled auto-merge (rebase) December 5, 2025 14:07
Use a simple fail jump instead of trying to do the cleanup at every return.

This catches a few error exits that were not calling tz_revert and/or where
not closing "out". Having so many separate returns was error prone.
@bjoernricks bjoernricks force-pushed the smaller-print-report-6 branch from 3ff7c6e to cdc0dd3 Compare December 8, 2025 07:14
@greenbonebot greenbonebot merged commit 5f62130 into main Dec 8, 2025
29 of 30 checks passed
@greenbonebot greenbonebot deleted the smaller-print-report-6 branch December 8, 2025 07:19
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.

3 participants