Closed Bug 1246619 Opened 5 years ago Closed 5 years ago

NSS clang-format: lib/freebl

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(firefox47 affected)

RESOLVED FIXED
Tracking Status
firefox47 --- affected

People

(Reporter: franziskus, Assigned: franziskus)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Applying clang-format to lib/freebl.
Attached patch LibFreeblClangFormat.patch (obsolete) — Splinter Review
clang-format on lib/freebl. patch is also at https://codereview.appspot.com/290100043

this is a big one. And for some reason clang-format did many strange line breaks. I tried to fix them but not sure if I spotted all. The patch also contains some clang-format protections. There was other code with special formatting, but I preferred the new format.
Flags: needinfo?(kaie)
Attachment #8716923 - Attachment is obsolete: true
Flags: needinfo?(kaie)
It would be nice to make progress on this one.
Attached patch freebl-cf.patchSplinter Review
so this is a big one again.

There was quite some custom formatting in here (a lot of the algorithms), but nothing I found worthy of keeping.
Attachment #8785241 - Flags: review?(kaie)
this patch enables clang-format on freebl on TC CI
Attachment #8785242 - Flags: review?(kaie)
Attachment #8785242 - Flags: review?(kaie) → review+
Comparison of freebl libs shows they're identical with and without the patch.
Comment on attachment 8785241 [details] [diff] [review]
freebl-cf.patch

The only unexpected change I saw, clang-format replaces tabs with spaces inside of strings!

In two places, it's done for the usage string of the ecp_test/ecperf tools. I think we don't need to worry about the small change of the help/usage output this change will cause.

In one place it replaced the string given to an __asm("...") statement.
  __asm("bswap{tab}%0....);

I seems that doesn't change the resulting, so it's ok with me.

r=kaie
Attachment #8785241 - Flags: review?(kaie) → review+
Target Milestone: --- → 3.27
(In reply to Kai Engert (:kaie) from comment #6)
> Comment on attachment 8785241 [details] [diff] [review]
> freebl-cf.patch
> 
> The only unexpected change I saw, clang-format replaces tabs with spaces
> inside of strings!
> 
> In two places, it's done for the usage string of the ecp_test/ecperf tools.
> I think we don't need to worry about the small change of the help/usage
> output this change will cause.
> 
> In one place it replaced the string given to an __asm("...") statement.
>   __asm("bswap{tab}%0....);
> 
> I seems that doesn't change the resulting, so it's ok with me.
> 
> r=kaie

Thanks Kai! right, the changes in __asm don't change anything and there aren't actually any output strings that are used (ecp_test is dead code).
You need to log in before you can comment on or make changes to this bug.