Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate SbStringFormat functions #1999

Merged
merged 3 commits into from
Dec 13, 2023
Merged

Deprecate SbStringFormat functions #1999

merged 3 commits into from
Dec 13, 2023

Conversation

maxz-lab
Copy link
Contributor

@maxz-lab maxz-lab commented Nov 17, 2023

Replace SbStringFormat*() calls with POSIX vsnprintf().
SbStringFormat -> vsnprintf
SbStringFormatF -> snprintf
SbStringFormatWide -> vswprintf
SbStringFormatUnsafeF -> sprintf

Backward compatibility is protected by "SB_API_VERSION < 16".

Tested on Linux, Android, Evergreen(14,15,16), and Win32.

b/307941391

Change-Id: I437ff3d7bf735b46767867ea844aeffb28e58b4d

@datadog-cobalt-youtube
Copy link

datadog-cobalt-youtube bot commented Nov 17, 2023

Datadog Report

Branch report: maxz-sb16-printfs
Commit report: 29fad93

❄️ cobalt: 0 Failed, 2 New Flaky, 29443 Passed, 1 Skipped, 35m 42.82s Wall Time

New Flaky Tests (2)

  • DrainingSlot/0 - SlotManagementTests/SlotManagementTest - Last Failure

    Expand for error
     ../../starboard/loader_app/slot_management_test.cc:136
     Expected equality of these values:
       exists
         Which is: false
       SbFileExists(bad_app_key_file_path.c_str())
         Which is: true
    
  • DrainingSlot/1 - SlotManagementTests/SlotManagementTest - Last Failure

    Expand for error
     ../../starboard/loader_app/slot_management_test.cc:136
     Expected equality of these values:
       exists
         Which is: false
       SbFileExists(bad_app_key_file_path.c_str())
         Which is: true
    

@andrewsavage1
Copy link
Contributor

Are you waiting for a review from me? Once you are, please remove WIP from the title of the PR

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d481619) 58.65% compared to head (9166e27) 58.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1999      +/-   ##
==========================================
+ Coverage   58.65%   58.66%   +0.01%     
==========================================
  Files        1906     1902       -4     
  Lines       94063    94044      -19     
==========================================
+ Hits        55173    55174       +1     
+ Misses      38890    38870      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kaidokert
Copy link
Member

I recommend converting this to draft for now, until majority builds pass.

@maxz-lab maxz-lab changed the title WIP: Deprecate SbStringFormat functions Deprecate SbStringFormat functions Nov 22, 2023
@maxz-lab maxz-lab marked this pull request as draft November 22, 2023 17:26
@maxz-lab
Copy link
Contributor Author

maxz-lab commented Nov 22, 2023

Are you waiting for a review from me? Once you are, please remove WIP from the title of the PR

Apologize for adding you in a bit too soon. I actually did not know how it happened.

@maxz-lab maxz-lab requested a review from y4vor November 22, 2023 17:30
@maxz-lab maxz-lab force-pushed the maxz-sb16-printfs branch 3 times, most recently from 29261d6 to be7c320 Compare November 22, 2023 19:03
@maxz-lab maxz-lab removed the request for review from y4vor November 22, 2023 19:07
Replace SbStringFormat*() calls with POSIX vsnprintf().
  SbStringFormat        -> vsnprintf
  SbStringFormatF       -> snprintf
  SbStringFormatWide    -> vswprintf
  SbStringFormatUnsafeF -> sprintf

Backward compatibility is protected by "SB_API_VERSION < 16".
Add vfwprintf to c99 allowed list.

Tested on Linux, Android, Evergreen(14,15,16), Win32

b/307941391

Change-Id: I437ff3d7bf735b46767867ea844aeffb28e58b4d
@maxz-lab maxz-lab force-pushed the maxz-sb16-printfs branch 6 times, most recently from 29fad93 to 2cce32c Compare November 27, 2023 18:14
@datadog-cobalt-youtube
Copy link

datadog-cobalt-youtube bot commented Nov 27, 2023

Datadog Report

Branch report: maxz-sb16-printfs
Commit report: ea95c0a

cobalt: 0 Failed, 0 New Flaky, 29711 Passed, 1 Skipped, 11m 20.35s Wall Time

@maxz-lab maxz-lab force-pushed the maxz-sb16-printfs branch 4 times, most recently from 36337c5 to 79029cf Compare November 28, 2023 17:01
starboard/nplb/string_format_test.cc Outdated Show resolved Hide resolved
starboard/nplb/string_format_wide_test.cc Outdated Show resolved Hide resolved
starboard/shared/stub/string_format.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@andrewsavage1 andrewsavage1 left a comment

Choose a reason for hiding this comment

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

Please make sure to test internally as well

base/strings/string_util_starboard.h Show resolved Hide resolved
base/strings/stringprintf.cc Outdated Show resolved Hide resolved
starboard/shared/win32/string_format.cc Outdated Show resolved Hide resolved
starboard/string.h Outdated Show resolved Hide resolved
starboard/string.h Outdated Show resolved Hide resolved
third_party/mini_chromium/base/strings/stringprintf.cc Outdated Show resolved Hide resolved
third_party/musl/src/starboard/stdio/vswprintf.c Outdated Show resolved Hide resolved
url/url_canon_internal.cc Outdated Show resolved Hide resolved
@maxz-lab maxz-lab force-pushed the maxz-sb16-printfs branch 4 times, most recently from 27d9298 to 2ad46d5 Compare December 7, 2023 23:07
@datadog-cobalt-youtube
Copy link

datadog-cobalt-youtube bot commented Dec 12, 2023

Datadog Report

Branch report: maxz-sb16-printfs
Commit report: 9166e27

cobalt: 0 Failed, 0 New Flaky, 29774 Passed, 1 Skipped, 11m 11.28s Wall Time

@maxz-lab maxz-lab force-pushed the maxz-sb16-printfs branch 2 times, most recently from 0c36f2f to 5a40115 Compare December 12, 2023 22:31
base/strings/string_util.h Outdated Show resolved Hide resolved
base/strings/string_util.h Outdated Show resolved Hide resolved
base/strings/string_util_starboard.h Show resolved Hide resolved
@maxz-lab maxz-lab force-pushed the maxz-sb16-printfs branch 2 times, most recently from 460d537 to 40a85a4 Compare December 13, 2023 06:29
base/strings/string_util.h Outdated Show resolved Hide resolved
Change-Id: I3c46ae6ee86bd60433ea15864bf799aa35205a63
@maxz-lab maxz-lab merged commit 35fd0c5 into main Dec 13, 2023
328 of 332 checks passed
@maxz-lab maxz-lab deleted the maxz-sb16-printfs branch December 13, 2023 18:33
isarkis added a commit that referenced this pull request Dec 13, 2023
isarkis added a commit that referenced this pull request Dec 13, 2023
Reverts #1999 as it broke XB1 and PS5

b/316211534
Rongo-JL pushed a commit to Rongo-JL/cobalt that referenced this pull request Dec 19, 2023
Replace SbStringFormat*() calls with POSIX vsnprintf().
  SbStringFormat        -> vsnprintf
  SbStringFormatF       -> snprintf
  SbStringFormatWide    -> vswprintf
  SbStringFormatUnsafeF -> sprintf

Backward compatibility is protected by "SB_API_VERSION < 16".

Tested on Linux, Android, Evergreen(14,15,16), and Win32.

b/307941391

Change-Id: I437ff3d7bf735b46767867ea844aeffb28e58b4d
Rongo-JL pushed a commit to Rongo-JL/cobalt that referenced this pull request Dec 19, 2023
dahlstrom-g pushed a commit that referenced this pull request Nov 26, 2024
Roll src/internal in M126 from 13a6bad1a34c to 52a6f9851523
Commits rolled:
https://chrome-internal.googlesource.com/chrome/src-internal.git/+log/13a6bad1a34c..52a6f9851523

Generated by: http://go/bbid/8731430505343673841

Change-Id: I9f0764cf77b1b95593b2dabfbcb81d776e5625d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6015060
Bot-Commit: Chrome Release Bot (LUCI) <chrome-official-brancher@chops-service-accounts.iam.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/6478@{#1999}
Cr-Branched-From: e6143acc03189c5e52959545b110d6d17ecd5286-refs/heads/main@{#1300313}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants