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

my_vsnprintf behaves not as in C standard

Details

    Description

      Some developers apparently don't like that now my_vsnprintf("%.*s") behaves not as in the C standard.

      So, we'll change it back and introduce %S to print the truncation indicator.

      All %s that print something to the end user have to be changed, for example, to %S. This includes all error messages and everything in the code too.

      Don't forget to update the documentation in the service header file.

      And we'll need some way to ensure that in the future all new printfs to the use also use %S.

      Attachments

        Issue Links

          Activity

            serg Sergei Golubchik created issue -
            serg Sergei Golubchik made changes -
            Field Original Value New Value
            Description Some developers apparently don't like that now {{my_vsnprintf("%.*s")}} behaves not as in the C standard.

            So, we'll change it back and introduce {{%S}} to print the truncation indicator.

            All {{%s}} that print something to the end user have to be changed to {{%S}}. This includes all error messages and everything in the code too.

            And we'll need some way to ensure that in the future all new printfs to the use also use {{%S}}.
            Some developers apparently don't like that now {{my_vsnprintf("%.*s")}} behaves not as in the C standard.

            So, we'll change it back and introduce {{%S}} to print the truncation indicator.

            All {{%s}} that print something to the end user have to be changed to {{%S}}. This includes all error messages and everything in the code too.

            Don't forget to update the documentation in the service header file.

            And we'll need some way to ensure that in the future all new printfs to the use also use {{%S}}.
            wlad Vladislav Vaintroub added a comment - - edited

            Please, do not use %S , this is confusing. %S It is there to print wide strings, i.e wchar_t *

            Even on Linux, and it is documented.

            wlad Vladislav Vaintroub added a comment - - edited Please, do not use %S , this is confusing. %S It is there to print wide strings, i.e wchar_t * Even on Linux, and it is documented.
            serg Sergei Golubchik made changes -
            Description Some developers apparently don't like that now {{my_vsnprintf("%.*s")}} behaves not as in the C standard.

            So, we'll change it back and introduce {{%S}} to print the truncation indicator.

            All {{%s}} that print something to the end user have to be changed to {{%S}}. This includes all error messages and everything in the code too.

            Don't forget to update the documentation in the service header file.

            And we'll need some way to ensure that in the future all new printfs to the use also use {{%S}}.
            Some developers apparently don't like that now {{my_vsnprintf("%.*s")}} behaves not as in the C standard.

            So, we'll change it back and introduce {{%S}} to print the truncation indicator.

            All {{%s}} that print something to the end user have to be changed, for example, to {{%S}}. This includes all error messages and everything in the code too.

            Don't forget to update the documentation in the service header file.

            And we'll need some way to ensure that in the future all new printfs to the use also use {{%S}}.
            wlad Vladislav Vaintroub added a comment - - edited

            Maybe this can change, for example, to %sT , where T stands for truncated. And yes, the output will be different from whatever standard says, but it does not use either new unknown format specifiers, nor does it hijack the already known and documented ones, and it is still compatible with string, so you can use _attribute(format) on that.

            wlad Vladislav Vaintroub added a comment - - edited Maybe this can change, for example, to %sT , where T stands for truncated. And yes, the output will be different from whatever standard says, but it does not use either new unknown format specifiers, nor does it hijack the already known and documented ones, and it is still compatible with string, so you can use _ attribute (format) on that.

            yes, %sT, but that's after MDEV-21978. Depends on what's implemented first.

            serg Sergei Golubchik added a comment - yes, %sT , but that's after MDEV-21978 . Depends on what's implemented first.
            serg Sergei Golubchik made changes -

            As far as I understand, the regression was introduced in MDEV-20604.

            marko Marko Mäkelä added a comment - As far as I understand, the regression was introduced in MDEV-20604 .
            marko Marko Mäkelä made changes -
            marko Marko Mäkelä made changes -
            Affects Version/s 10.5.3 [ 24263 ]
            Affects Version/s 10.4.13 [ 24223 ]
            Affects Version/s 10.3.23 [ 24222 ]
            Affects Version/s 10.2.32 [ 24221 ]
            Affects Version/s 10.2 [ 14601 ]
            Affects Version/s 10.3 [ 22126 ]
            Affects Version/s 10.4 [ 22408 ]
            Affects Version/s 10.5 [ 23123 ]
            sanja Oleksandr Byelkin made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            sanja Oleksandr Byelkin made changes -
            Priority Major [ 3 ] Blocker [ 1 ]
            sanja Oleksandr Byelkin made changes -
            Assignee Oleksandr Byelkin [ sanja ] Sergei Golubchik [ serg ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            serg Sergei Golubchik made changes -
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Oleksandr Byelkin [ sanja ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            marko Marko Mäkelä added a comment - - edited

            Sorry, I needed 2 after-merge fixes on 10.4. For the first one, my Windows build had failed without me noticing it, and that is why the %s→%T change was not reflected in the result file.

            marko Marko Mäkelä added a comment - - edited Sorry, I needed 2 after- merge fixes on 10.4. For the first one, my Windows build had failed without me noticing it, and that is why the %s→%T change was not reflected in the result file.
            sanja Oleksandr Byelkin made changes -
            Fix Version/s 10.3.24 [ 24306 ]
            Fix Version/s 10.4.14 [ 24305 ]
            Fix Version/s 10.2.34 [ 24505 ]
            Fix Version/s 10.2 [ 14601 ]
            Fix Version/s 10.3 [ 22126 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            sanja Oleksandr Byelkin made changes -
            Fix Version/s 10.5.4 [ 24264 ]
            ralf.gebhardt Ralf Gebhardt made changes -
            Fix Version/s 10.2.35 [ 25022 ]
            ralf.gebhardt Ralf Gebhardt made changes -
            Fix Version/s 10.2.34 [ 24505 ]
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 108626 ] MariaDB v4 [ 157762 ]

            People

              sanja Oleksandr Byelkin
              serg Sergei Golubchik
              Votes:
              0 Vote for this issue
              Watchers:
              4 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.