Evan Layton wrote:
>
>>>
>>>> 3) In log_bld_info(), you used temp file to capture the output of
>>>> "pkg.....". I think it is much simpler
>>>> to use "popen()" to execute the command and get the output and
>>>> completely avoid the temp file.
>>>
>>> definitely not simpler... ;-)
>>>
>>> While this is a bit complicated I really don't want to have to
>>> figure out the pid, do a popen(), attach the pipe, run system() and
>>> then grab the output from the package command. I'd much rather leave
>>> it the way it is...
>> Actually, looking at the popen(3C) man page, or googling for popen
>> gives many examples on how to use
>> it. Why does it involve figuring out the pid, attaching the pipe and
>> running system() in this case?
>
> umm you're right, it doesn't...
>
> I guess I could change this to use something more like:
>
> void
> log_bld_info(char *mntpnt, char *comment)
> {
> char rel_str[BUFSIZ] = {0};
> char cmd[MAXPATHLEN] = {0};
> FILE *fp = NULL;
> int fd = 0, i = 0;
>
> snprintf(cmd, MAXPATHLEN, "%s info entire | grep FMRI",
> PKG_PATH);
>
> if ((fp = popen(cmd, "r")) != NULL) {
> if (fgets(rel_str, BUFSIZ, fp) != NULL)
> om_log_print("%s\n%s\n", comment, rel_str);
> (void) pclose(fp);
> return;
> }
>
> om_debug_print(OM_DBGLVL_WARN, "log_bld_info: Unable to "
> "retrieve build version information\n");
> return;
>
> }
Yeah, that looks much simpler to me than your original code with the
temp file.
If I were writing the code, I would even skip the " | grep FMRI" part
and do the "grep" equivalent while I process the strings the "fgets", but
I guess that's just a personal style.
In your code above, do we also want to log the warning if we can't find
the FMRI string? The way the code is written above, it won't log a warning
for that, and I think it should.
Thanks,
--Karen
