[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2
From: |
Markus Mützel |
Subject: |
[Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2 |
Date: |
Thu, 10 Nov 2022 11:53:27 -0500 (EST) |
Update of patch #10292 (project octave):
Status: None => In Progress
_______________________________________________________
Follow-up Comment #2:
Thank you very much for the patch.
I haven't tested it yet. Just a couple of remarks:
* I see you moved including the header from the .cc file(s) to the .h file.
Would it be possible keep them in their old locations? That would reduce the
number of (unnecessary) symbols declared in user code. Or is there a reason
for this change?
* In lines 227 and following of your patch: I don't understand why `start` and
`end` are of type double to begin with. You changed it to type int. Shouldn't
it be of type std::size_t (or maybe better yet of type PCRE2_SIZE) instead? I
guess that would also apply to the respective properties of
`regexp::match_element`. Maybe that should be a separate change. But feel free
to include it here if you prefer.
* In lines 288 and 289: Static_casting between integer and floating-point
types might be risky for large values (that can be represented in one type but
not the other). Should there be more thorough checks? (That point might be
mute if we change the type of those properties and the corresponding
constructor in `regexp::match_element`.)
* In the code, you #define `PCRE2_DATA_WIDTH` and `PCRE2_CODE_UNIT_WIDTH`.
Should both also be defined for the configure check?
* Instead of repeating `pcre2_match_data_free` in several locations (which
could easily be missed in a future change), could you use `unwind_action`
instead? See, e.g., liboctave/util/oct-glob.cc:90:
void *glob_info = octave_create_glob_info_struct ();
unwind_action cleanup_glob_info_struct
([=] () { octave_destroy_glob_info_struct (glob_info); });
_______________________________________________________
Reply to this item at:
<https://savannah.gnu.org/patch/?10292>
_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Rafael Laboissière, 2022/11/10
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, John W. Eaton, 2022/11/10
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2,
Markus Mützel <=
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Rafael Laboissière, 2022/11/10
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Markus Mützel, 2022/11/10
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Rafael Laboissière, 2022/11/11
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Markus Mützel, 2022/11/11
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, John W. Eaton, 2022/11/11
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, John W. Eaton, 2022/11/13
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Rafael Laboissière, 2022/11/13
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Rafael Laboissière, 2022/11/14
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Markus Mützel, 2022/11/15
- [Octave-patch-tracker] [patch #10292] Switch from PCRE to PCRE2, Markus Mützel, 2022/11/15