Skip to content
This repository was archived by the owner on Jan 31, 2020. It is now read-only.

Check if GLOB_BRACE is defined before using it #58 #59

Merged
merged 1 commit into from
Apr 12, 2016
Merged

Check if GLOB_BRACE is defined before using it #58 #59

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Glob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected static function systemGlob($pattern, $flags)
self::GLOB_NOSORT => GLOB_NOSORT,
self::GLOB_NOCHECK => GLOB_NOCHECK,
self::GLOB_NOESCAPE => GLOB_NOESCAPE,
self::GLOB_BRACE => GLOB_BRACE,
self::GLOB_BRACE => defined('GLOB_BRACE') ? GLOB_BRACE : 0,
Copy link
Member

Choose a reason for hiding this comment

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

Does GLOB_BRACE always default to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think so, because it is a flag.

https://github.com/php/php-src/blob/1c295d4a9ac78fcc2f77d6695987598bb7abcb83/ext/standard/dir.c#L162-L166

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, I've tested it, the error went away and it did its job. :)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but the flag comes from the underlying lib, not from PHP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before posting the issue here, I've checked some approaches taken by other softwares and they did the same and defaulted it to 0.

http://git.alpinelinux.org/cgit/aports/plain/main/asterisk/musl-glob-compat.patch
http://lists.alpinelinux.org/alpine-aports/1379.html

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for digging out the links!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're welcome. Wouldn't you please mind to notice me when the next release with this is launched? Because I've did that change on a docker container after running compose and I will need to remove it later.

Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

@cusspvz you should get a notification when this is merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, so you land all PRs just before releasing the version? I'm used to get them landed and wait 'til the next release, thats why I've requested you to notice me. 👍

Copy link
Member

Choose a reason for hiding this comment

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

@cusspvz We're now typically doing bugfix releases immediately following merges. 😄

self::GLOB_ONLYDIR => GLOB_ONLYDIR,
self::GLOB_ERR => GLOB_ERR,
];
Expand Down