On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote: > > #define HEADER_TOKEN "X-Do" /* the HTTP header for the auth token */ > "cker-Token:" > -#define HEADER_REGISTRY "X-Do" /*the HTTP header for the registry */ > "cker-Endpoints:" > +#define HEADER_REGISTRY "X-Do" /* the HTTP header for the registry */ > "cker-Endpoints:" > +#define HEADER_DIGEST "Do" /* the HTTP header for the manifest digest */ > "cker-Content-Digest:" > +#define HEADER_USER_AGENT_V2 "User-Agent: do" /* otherwise we get > load-balanced(!) to a V1 registyry */ "cker/1.6.0"
I hope we can get rid of this. I'd prefer if we wouldn't have to pretend to be other software. > +#define HEADER_BEARER_REALM "https://auth.doc"; /* URL which we query for a > bearer token */ "ker.io/token" > +#define HEADER_BEARER_SERVICE "registry.doc" /* the service we > query the token for */ "ker.io" These two are problematic, we really shouldn't hardcode them. First of all users might want to use other servers here. More importantly though, to my knowledge the service agreements of the company providing those server does not allow usage like this. IANAL, but I really would prefer staying out of any legal issues here. We can make the defaults for this configurable via ./configure switches but we should not carry these URLs by default. If downstream distros are willing to take the legal risk of encoding the URLs by default then, they are welcome to via the configure switches, but upstream I really don't want to see that... Also, can you quickly explain how the three servers relate to each other? Is this documented anywhere? > > + if (strategy == PULL_V2) { > + char **k = NULL; > + STRV_FOREACH(k, i->ancestry) { > + char *d = strjoina(i->image_root, "/.dkr-", > *k, NULL); Using alloca() (which strjoina() uses internally) within loops is not OK, since deallacation happens at the end of the function call, not at the end of the {} block. > + > + r = json_parse((const char *)j->payload, &doc); Why the cast? void, and non-const automatically upgrade to const and any type... > > +enum PullStrategy { PULL_V1, PULL_V2 }; So far we typedef'ed enums for exported types, and prefixed both with a namespacing prefix. Also, why call this a "strategy"? Does dkr upstream call it that way? Shouldn't this be "protocol" or so? typedef enum DkrProtocolVersion { DKR_PROTOCOL_V1, DKR_PROTOCOL_V2, } DkrProtocolVersion; > + > +typedef struct DkrSignature { > + char *curve; > + > + char *key_id; > + char *key_type; > + > + char *x; > + char *y; > + > + char *algorithm; > + > + char *signature; > + char *protected; > +} DkrSignature; This is not used so far, is it? > + > +//typedef struct DkrHistory { > +// char *image_id; > +// char *parent_id; > +//} DkrHistory; And neither is this? Also, we don't use C++ comments in our code, i.e. /* this */ is preferred over // this... Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
