Looks okay to me too.
-ethan
Karen Tung wrote:
> 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
>>>
>>>
>>>
>>
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
