Re: [Patch] Service Fingerprints in XML

[Brandon Enright <bmenrigh () ucsd edu>]

On Tue, 2006-04-18 at 00:16 -0700, Fyodor wrote:
> Hi Brandon.  Thanks for the patch.  It looks pretty good and I'd like
> to add it, but I have a couple concerns:
>
> > +  if (sd->service_fp) {
> > +    xml_servicefp = xml_sf_convert(xml_convert(sd->service_fp));
> > +    versionxmlstring += " servicefp=\"";
> > +    versionxmlstring += xml_servicefp;
> > +    free(xml_devicetype); xml_servicefp = NULL;
> > +    versionxmlstring += '\"';
> > +  }
>
> Do you mean to free(xml_servicefp) here instead of xml_devicetype?

Yes.  Not sure how I let that one slip by.

> > +/* Remove all "\nSF:" from fingerprints */
> > +char* xml_sf_convert (const char* str) {
>
> I think there is room to improve this implementation.  It copies the
> string a whole lot of times, and has some variables (such as substr[]
> which don't seem necessary).  Couldn't you do something like:
>
> char *temp = (char *) safe_malloc(strlen(str) + 1);
> char *dst = temp, *src=str;
> while(*src) {
>    if (strcmp(src, "\nSF:") == 0) {
>      src += 4;
>    }
>    if (*src)
>      *dst++ = *src++;
> }
> *dst = '\0';
>
> Or am I missing some reason why this won't work?  Do you think you
> could make and test these changes and then resend your patch?

I don't see any reason why you're method wouldn't work and it is vastly
superior to my convoluted method of doing it.  Obviously my C/C++ is a
little rusty.

> Thanks,
> Fyodor


I'll clean this patch up and post it again.  Thanks for the feedback.

Brandon



_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev