On Fri, Dec 14, 2007 at 11:53:35PM +0100, Jim Meyering wrote: > "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: > > On Fri, Dec 14, 2007 at 08:34:13AM +0100, Jim Meyering wrote: > >> Daniel Veillard <veillard@xxxxxxxxxx> wrote: > >> > On Thu, Dec 13, 2007 at 11:21:31PM +0100, Jim Meyering wrote: > >> >> "Richard W.M. Jones" <rjones@xxxxxxxxxx> wrote: > >> >> ... > >> >> > + * virBufferURIEncodeString: > >> >> > + * @buf: the buffer to append to > >> >> > + * @str: the string argument which will be URI-encoded > >> >> > + * > >> >> > + * Append the string to the buffer. The string will be URI-encoded > >> >> > + * during the append (ie any non alpha-numeric characters are replaced > >> >> > + * with '%xx' hex sequences). > >> >> > + * > >> >> > + * Returns 0 successful, -1 in case of internal or API error. > >> >> > + */ > >> >> > +int > >> >> > +virBufferURIEncodeString (virBufferPtr buf, const char *str) > >> >> > +{ > >> >> > + int grow_size = 0; > >> >> > + const char *p; > >> >> > + unsigned char uc; > >> >> > + const char *hex = "0123456789abcdef"; > >> >> > + > >> >> > + for (p = str; *p; ++p) { > >> >> > + /* Want to leave only strict 7 bit ASCII alphanumerics ... */ > >> >> > + if ((*p >= '0' && *p <= '9') || > >> >> > + (*p >= 'a' && *p <= 'z') || > >> >> > + (*p >= 'A' && *p <= 'Z')) > >> >> ... > >> >> > + for (p = str; *p; ++p) { > >> >> > + if ((*p >= '0' && *p <= '9') || > >> >> > + (*p >= 'a' && *p <= 'z') || > >> >> > + (*p >= 'A' && *p <= 'Z')) > >> >> > >> >> Hi Rich, > >> >> > >> >> What do you think of using this? > >> >> > >> >> isascii (*p) && isalnum (*p) > >> > > >> > I have learned to be very cautious of the is* macros because they > >> > tend to be local dependant whichis usually really not what you would > >> > like or expect. In that case this may work, but explicit ranges > >> > are 100% clear about what you intend to accept or not, that's why I > >> > usually prefer them. > >> > >> You're right that we shouldn't use isalnum here. > >> However, we shouldn't use inequality comparisons, either. > >> While 0 <= c <= 9 is guaranteed to be ok for the digits, the > >> a..z and A..Z ranges need not be contiguous, i.e., with EBCDIC: > > > > Seriously who gives a damn about EBCDIC anymore. > > Daniel and Daniel, > > Personally, I don't care at all, but I suspect that people using z/OS > and OS/390 care, among others. Some might even be Red Hat customers. > What's wrong with making the code portable, just in case? Portability while certainly worthwhile isn't absolute - we only have finite development time. While we could spend that time worrying about EBCDIC and other platforms we're not likely to encounter any time soon, there is a tradeoff to be made at some point where we have to get back to actually developing features that matter to the 99.99999% of our userbase who isn't EBCDIC. I'm not saying we should go out of our way to break other platforms, but there's a cost/benefit tradeoff to be had. > > This just makes it totally unreadable. > > This just makes *what* totally unreadable? The giant switch statements is what I was referring to. > I proposed to do this: > > for (p = str; *p; ++p) { > is_alphanum (*p) > ... > > for (p = str; *p; ++p) { > is_alphanum (*p) I've no objection to this - just the giant switch statement. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list