...
<rdar://problem/48147968>
Created attachment 362264 [details] Patch
Comment on attachment 362264 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362264&action=review > Source/JavaScriptCore/runtime/CodeCache.h:152 > + close(fd); Nit: I’d just create an onScopeExit after the branch to make sure it’s a valid fd. I’d also remove the other close(fd)
(In reply to Saam Barati from comment #3) > Comment on attachment 362264 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=362264&action=review > > > Source/JavaScriptCore/runtime/CodeCache.h:152 > > + close(fd); > > Nit: I’d just create an onScopeExit after the branch to make sure it’s a > valid fd. I’d also remove the other close(fd) Not sure what do you mean by making sure it's a valid fd after the branch, we always check and return. I tried to find a helper like that before, but never did. Grepping for `onScopeExit` didn't find anything either.
Comment on attachment 362264 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362264&action=review >>> Source/JavaScriptCore/runtime/CodeCache.h:152 >>> + close(fd); >> >> Nit: I’d just create an onScopeExit after the branch to make sure it’s a valid fd. I’d also remove the other close(fd) > > Not sure what do you mean by making sure it's a valid fd after the branch, we always check and return. I tried to find a helper like that before, but never did. Grepping for `onScopeExit` didn't find anything either. What I’m proposing is: On line 137, add: auto closeFD = makeScopeExit([&] { close(fd); }) I misremembered the name of the function
(In reply to Saam Barati from comment #5) > Comment on attachment 362264 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=362264&action=review > > >>> Source/JavaScriptCore/runtime/CodeCache.h:152 > >>> + close(fd); > >> > >> Nit: I’d just create an onScopeExit after the branch to make sure it’s a valid fd. I’d also remove the other close(fd) > > > > Not sure what do you mean by making sure it's a valid fd after the branch, we always check and return. I tried to find a helper like that before, but never did. Grepping for `onScopeExit` didn't find anything either. > > What I’m proposing is: On line 137, add: > auto closeFD = makeScopeExit([&] { close(fd); }) > > I misremembered the name of the function Nice! Thanks, I'll update it.
Created attachment 362266 [details] Patch
Comment on attachment 362266 [details] Patch Clearing flags on attachment: 362266 Committed r241660: <https://trac.webkit.org/changeset/241660>
All reviewed patches have been landed. Closing bug.