On Mon, Aug 14, 2006 at 12:34:56PM -0400, Alan Stern wrote:
> Does anyone object to the patch below? It would help at least one person;
> I'm concerned that it might cause trouble somewhere else.
> @@ -526,6 +519,12 @@ static int scsi_probe_lun(struct scsi_de
> if (response_len > 255)
> response_len = first_inquiry_len; /* sanity */
>
> + /* Sanitize the Vendor, Product, and Revision fields. */
> + for (i = 8; i < 36; ++i) {
> + if (inq_result[i] < 0x20 || inq_result[i] > 0x7e)
> + inq_result[i] = ' ';
> + }
> +
> /*
If we have one non-printable character, should we not overwrite all
subsequent characters with spaces? I'm concerned that they may
NUL-terminate a string, then the remnants be garbage. So in other
words, I'd like to see something like:
void scsi_inquiry_sanitise(unsigned char *inquiry, int start, int end)
{
int i, unprintable = 0;
for (i = start; i < end; i++) {
unprintable |= (inq_result[i] < 0x20 || inq_result[i] > 0x7e)
if (unprintable)
inq_result[i] = ' ';
}
}
and then call it:
scsi_inquiry_sanitise(inq_result, 8, 16);
scsi_inquiry_sanitise(inq_result, 16, 32);
scsi_inquiry_sanitise(inq_result, 32, 36);
This of course begs the questions:
- Do we want to mangle the inquiry return like this?
- Should we simply copy out the strings from the raw inquiry result, and
fix them at copy time
- What about someone who just put a tab in there and now gets
everything overwritten with spaces
That last one could be fixed with a more complex function ...
void scsi_inquiry_sanitise(unsigned char *inquiry, int start, int end)
{
int i, terminated = 0;
for (i = start; i < end; i++) {
terminated |= (inq_result[i] == 0)
if (terminated || inq_result[i] < 0x20 || inq_result[i] > 0x7e)
inq_result[i] = ' ';
}
}
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
