> That's always a good start! Where is this repository that your patch is > against?
http://cvs.savannah.gnu.org/viewvc/xmlfs/?root=hurdextras > Your editor / mailer broke the lines, so this patch won't even apply. > Either you instruct your editor / mailer, or let git send-email take care > of that, or attach the patches instead of publishing them inline. Heh, oops. I forgot claws-mail strictly enforces line length limits. I'll keep that in mind > Hrm. Not sure whether we'd like all of these. But you're (to the best > of my knowledge) to only person working on xmlfs these days, and we have > no general template, so I'd let you decide. If any of them turn out to be a problem, or overly complicate the code (as pretty much everything needs to be explicitly stated with those flags) I'll consider removing some of them. > If we really want this, wouldn't it be better to use ``__attribute__ > ((unused))'' with each of the functions' parameters? In Hurd code, we > can unconditionally use GNU C / GCC features. I didn't know of that attribute, and I think I will start using GCC/GNU stuff more (dropping -pedantic), as GCC is the compiler used (as antrik pointed out in another email) > I don't know the surrounding code yet, but should gsize perhaps by a > size_t instead? Probably, a lot of the 'problems' could undoubtedly have been fixed in other ways. > memcpy with length of one? Why not replace that with: > > data[size++] = '\n'; > > (Your cast of data doesn't look right anyway; do you understand and > agree?) I should have a look at the whole function -- it still looks a > bit suspicious: what will come after the '\n' charater; is a '\0' > expected there? Yes, that would be a much better solution. I didn't really read the code thoroughly enough when fixing the warnings, usually going for the most obvious solution (which may turn out to have caused some issues later down the line, I suppose). And yes, the case of data looks completely wrong *facepalm* > Why move these functions? Typically, all definitions should be as tight > in scope as possible. Warnings about nested functions, though if I'm going to drop -pedantic and use GCC extensions, that doesn't matter. > That doesn't look sane. According to netfs.h these indeed aren't const, > but why? They're used only in libnetfs/io-version.c. I'm not sure. I assumed there was a reason for libnetfs not having them as consts. > Both openport and open return an int file descriptor, so why is xmlfile a > file_t (which is another name for a mach_port_t)? Looks like xmlfs_create (in fs.c) is using a file_t as a file descriptor. As that's clearly wrong I'll change that. >> @@ -114,7 +118,7 @@ main (int argc, char **argv) >> >> netfs_root_node->nn_stat = underlying_stat; >> netfs_root_node->nn_stat.st_mode = >> - S_IFDIR | (underlying_stat.st_mode & ~S_IFMT & ~S_ITRANS); >> + S_IFDIR | (underlying_stat.st_mode & (unsigned int) ~S_IFMT & >> (unsigned int) ~S_ITRANS); > > What's the warning here? Implicit casting to unsigned int. Thanks for the comments. -- Michael Walker (http://www.barrucadu.co.uk) Arch Hurd Developer; GNU Webmaster; FSF member #8385 http://www.archhurd.org http://www.gnu.org http://www.fsf.org
