Hi Evan,
The updated webrev looks good to me.
Thanks for making all the changes.
--Karen
Evan Layton wrote:
> Karen Tung wrote:
>> Evan Layton wrote:
>>> Karen Tung wrote:
>>>> Yeah, that looks much simpler to me than your original code with
>>>> the temp file.
>>>
>>> agreed.
>>>
>>>> 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.
>>>
>>> I could definitely do that and just look for the FMRI string in a
>>> while loop but doing things the way they are makes it so I don't
>>> need to do the while loop. Did you want me to change this?
>>>
>> No need to change...
>>>>
>>>> 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.
>>>
>>> Oh I think I see what you mean. I should add a line to log this
>>> warning after the om_debug_print(). Something like:
>>>
>>> om_log_print("Warning: Unable to retrieve build version
>>> information\n");
>> Yeah, in an "else" statement for that "f (fgets(rel_str, BUFSIZ, fp)
>> != NULL) "
>
> Instead of doing an else I just did the following so I only doe the
> error logging once.
>
> if (fgets(rel_str, BUFSIZ, fp) != NULL)
> om_log_print("%s %s\n", comment, rel_str);
> (void) pclose(fp);
> return;
> }
> (void) pclose(fp);
>
> The webrev has been updated...
>
> Thanks again!
>
> -evan
>
>>
>> --Karen
>>
>>
>>
>
