octave-patch-tracker
[Top][All Lists]
Advanced

[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/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]