Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-20453

add class similar to std::string_view (non owning string reference)

Details

    Description

      Class is basically:

      class string_view {
      		 
        const char *string;
      		 
        size_t length;
      		 
      };

      It's intended to replace all naked char * and const std::string:: & in functions arguments.
      string_view is better because of a clear semantics: non-owning string. When you see a member or variable const char *smth; you can't say whether it own its buffer or not. For argument you don't know whether ownership was transferred to function or not. Similar to return char * from function.

      string_view makes code easier to read. Also it has begin(), end() member which allows it's data to be used in range-for loops.

      https://en.cppreference.com/w/cpp/string/basic_string_view

      Attachments

        Activity

          Ascending order - Click to sort in descending order
          marko Marko Mäkelä added a comment -

          It seems that the function fsp_path_to_space_name() is initially accessing one byte before the start of the string, in the first iteration of string_view::rfind() with the default parameter pos=npos, tripping AddressSanitizer in many tests. I reverted the change due to this. Please fix and test with ASAN.

          marko Marko Mäkelä added a comment - It seems that the function fsp_path_to_space_name() is initially accessing one byte before the start of the string, in the first iteration of string_view::rfind() with the default parameter pos=npos , tripping AddressSanitizer in many tests. I reverted the change due to this. Please fix and test with ASAN.
          marko Marko Mäkelä added a comment -

          The originally found issue may have been resolved by MDEV-25312, which removed the data member fil_space_t::name.

          I think that there needs to be some measurable benefit from such refactoring. I see that we currently use std::string (and are unnecessarily copying some data) in file_name_t::name and renamed_spaces. We could make those point directly to the redo log parse buffer. The data would only have to be copied at the end of each non-final recv_sys_t::apply() before the underlying buffer would be invalidated by recv_sys_t::free(). In that way, we would avoid copying any file name strings when only one recovery batch is needed.

          marko Marko Mäkelä added a comment - The originally found issue may have been resolved by MDEV-25312 , which removed the data member fil_space_t::name . I think that there needs to be some measurable benefit from such refactoring. I see that we currently use std::string (and are unnecessarily copying some data) in file_name_t::name and renamed_spaces . We could make those point directly to the redo log parse buffer. The data would only have to be copied at the end of each non-final recv_sys_t::apply() before the underlying buffer would be invalidated by recv_sys_t::free() . In that way, we would avoid copying any file name strings when only one recovery batch is needed.
          marko Marko Mäkelä added a comment -

          I think that this basically already exists as LEX_STRING and LEX_CSTRING. We perhaps should not develop several ways of doing the same thing, and I am afraid that there would be strong opposition if the existing classes were renamed or refactored.

          marko Marko Mäkelä added a comment - I think that this basically already exists as LEX_STRING and LEX_CSTRING . We perhaps should not develop several ways of doing the same thing, and I am afraid that there would be strong opposition if the existing classes were renamed or refactored.

          People

            kevg Eugene Kosov (Inactive)
            kevg Eugene Kosov (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Git Integration

                Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.