Hi Sundar,
I reviewed this... comments below:
auto_install.c:
configure_ips_init_nv_list
configure_ips_addl_publisher
you should check to be sure that the data, specifically auto_repo_info_t
* is not null.
auto_parse.c:
ai_get_manifest_repo_publisher()
> 535 + * If publisher is not supplied, check for authority
> 536 + */
> 537 + if (len <= 0) {
> 538 + if (is_default_publisher) {
> 539 + snprintf(tag, sizeof (tag),
> 540 + AIM_ADD_DEFAULT_URL_AUTH_NAME, url);
> 541 + } else {
> 542 + snprintf(tag, sizeof (tag),
> 543 + AIM_ADD_ADDL_URL_AUTH_NAME, url);
> 544 + }
> 545 + }
Don't we need to call ai_get_manifest_values(tag, &len) again here since
we are now looking for the authority as opposed to the publisher for
backward compatibility?
The addition of:
AIM_ADD_XXX macros in auto_install.h are actually more confusing to me.
I see what you are doing, but it seems confusing to see these, and
wonder why they are different than the previously defined AIM_IPS_XXX.
On this note.. the way we are specifying the tags to find seems wrong.
But, I may be wrong :-). Why do we need to need to specify this tag this
way:
> 188 +#define AIM_ADD_DEFAULT_URL_PUBLISHER_MIRROR \
> 189 + "ai_manifest/ai_pkg_repo_default_publisher/" \
> 190 + "main[url=\"%s\"]/../mirror/url"
In particular the traversing up one level in our specification with '..'.
thanks,
sarah
****
sundar Yamunachari wrote:
> Ethan, Alok and William,
>
> Please do a code review for the bugs 12659 and 13112. This is to allow
> more than one additional publishers to be specified with AI manifest.
>
> The webrev is at: http://cr.opensolaris.org/~ysundar/12659/
>
> The bugs are at http://defect.opensolaris.org/bz/show_bug.cgi?id=12659
> and http://defect.opensolaris.org/bz/show_bug.cgi?id=13112
>
> Thanks,
> Sundar
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
