[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

Fri Oct 28 15:05:08 PDT 2022

MaskRay accepted this revision.
MaskRay added inline comments.
Herald added a subscriber: StephenFan.

================
Comment at: llvm/lib/MC/MCContext.cpp:77
       AutoReset(DoAutoReset), TargetOptions(TargetOpts) {
-  SecureLogFile = AsSecureLogFileName;
+  SecureLogFile = TargetOptions ? TargetOptions->AsSecureLogFile : "";

----------------
benlangmuir wrote:
> benlangmuir wrote:
> > MaskRay wrote:
> > > When is TargetOptions null? When it is TargetOptions, Does a value of `""` matter?
> > > When is TargetOptions null? 
> > 
> > null is the default value for the parameter, and happens in various places that construct MCOptions. None of them require a secure log file as far as I can tell.
> > 
> > > Does a value of "" matter?
> > 
> > Like the other file path in MCTargetOptions  (SplitDwarfFile) empty indicates no file specified.
> @MaskRay was this just a question, or did you want me to change something here? Wanted to check before I merge this.
Just a question. This looks good.

The environment variable use is a bit odd. I do not know whether it is a legacy feature...

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136888/new/

https://reviews.llvm.org/D136888