Skip to content

Conversation

@Girgias
Copy link
Member

@Girgias Girgias commented Sep 28, 2024

No description provided.

ZEND_TRY_ASSIGN_REF_LONG(by_ref_line, line);
}

RETURN_BOOL(SG(headers_sent));
Copy link
Member

Choose a reason for hiding this comment

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

quick question: is there a reason why headers_sent global is not just a bool ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it hasn't been refactored since the move to C99, but it should yes.

@Girgias Girgias requested a review from ndossche December 28, 2025 00:24
$file = null;
$line = null;

$v1 = headers_sent(line: $line);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if using named params here (and later) was intentional?

@@ -0,0 +1,110 @@
--TEST--
headers_sent() by-ref argument with named arguments
Copy link
Member

Choose a reason for hiding this comment

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

This test file is a bit hard to read, maybe you should split the invocations with --- subtestname --- so that different subtests are easier to match between output and code.

file = php_output_get_start_filename();
}

switch(ZEND_NUM_ARGS()) {
Copy link
Member

Choose a reason for hiding this comment

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

I think the code was written this way because this avoids a check: if we have a second param then we know the first one was also passed. The new code doesn't avoid such a check. It probably doesn't really matter however as the new code produces less machine code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants